Re: [PATCH] add new module filevercmp

[Kamil Dudka]

Hello Jim,

thank you for review. I've made the changes you requested, new patch attached.


On Wednesday 24 September 2008 22:52:00 Jim Meyering wrote:
> > +  int rc = verrevcmp (s1, s1_len, s2, s2_len);
> > +  return (rc == 0)
> > +    /* return 0 if (and only if) strings S1 and S2 are identical */
> > +    ? strcmp(s1, s2) : rc;
>
> strcmp(s1, s2) is always nonzero here, since
> it was tested at the beginning of the function.
That's true, but I don't think here is a problem. Please consider new function 
description:
+   3) If both (PREFIX and  VERSION) are equal, strcmp function is used for
+      comparison. So this function can return 0 if (and only if) strings S1
+      and S2 are identical.

> > +  /* these results differ from strverscmp
> > +  ASSERT (filevercmp ("000", "00") < 0);
> > +  ASSERT (filevercmp ("00", "000") > 0); */
>
> Rather than just commenting out these two, instead, assert
> the correct-for-filevercmp condition and add a comment per line.
>
> > +  ASSERT (filevercmp ("a0", "a") > 0);
> > +  ASSERT (filevercmp ("00", "01") < 0);
> > +  ASSERT (filevercmp ("01", "010") < 0);
> > +  /* these results differ from strverscmp
> > +  ASSERT (filevercmp ("010", "09") < 0);
> > +  ASSERT (filevercmp ("09", "0") < 0); */
>
> Likewise.
I removed both of them completely since there are a bit confusing in the 
context of filevercmp. It ignores leading zeros and if the versions are 
equal, strcmp is used to distinguish theirs (deterministic) order.

Hopefully I didn't forget anything.


Kamil

gnulib-filevercmp.patch
Description: Text Data