Re: [PATCH 1/4] std.pk: Implement strrchr

[Jose E. Marchesi]

Hi Arsen.
Thanks for the patch.

> This patchset implements the pk_vercmp function, intended for comparing
> GNU poke version strings, as well as the requisite standard library
> facilities used in such a string manipulation heavy function.  The
> implementation in pk_vercmp is dual purpose, in that it both verifies
> and parses a version string.
>
> An alternative implementation could be written that populates a struct
> with broken out version information, rather than ad-hoc parsing two
> strings at once as is demonstrated here, as a pk_verparse, with
> pk_vercmp simply being a helper that returns the lexicographic
> comparison of two parsed versions.  I had initially opted to write a
> non-verifying lax version of this function, but bolted strictness onto
> it later, which is why this is the implementation I am presenting.  If
> you wish, I can upgrade to the approach described in this paragraph.
> Other patches are unaffected by this decision, and so, can be reviewed
> in either case.

I like the pk_parse_version approach you describe there.

The Pk_Version struct in pkl-rt could be something like:

type Pk_Version =
  struct
  {
    /* major.minor[.subminor][-BRANCH-OFFSET-HASH] */
    uint<8> major;
    uint<8> minor;
    uint<8> subminor;
    string branch : branch in ["dev", "maint"];
    uint<8> offset;
    string tag;
  };

And then we could have pk_ver_cmp = (Pk_Version a, Pk_Version b) int<32>.

>
> Are [1,3] OK for master?  What do you think about 4 (the discussion in
> the paragraph above?

Just this:

>  @menu
>  * ltrim::            Remove leading characters.
>  * rtrim::            Remove trailing characters.
> -* strchr::           Locate a character in a string.
> +* strchr::              Locate a character in a string, from the beginning.

Is the indentation there right?

> +* strrchr::          Locate a character in a string, from the end.
>  @end menu

Other than that, the strrchr patch is OK for master.
Thanks!