Re: [FEATURE_REQUEST] support openssl checksum format too

[Jim Meyering]

Guenter Knauf wrote:

> Jim,
> thanks for the very quick review.
>
> Jim Meyering schrieb:
>> Guenter Knauf wrote:
>>> -  size_t i;
>>> +  size_t i = 0;
>>>    bool escaped_filename = false;
>>>    size_t algo_name_len;
>>>
>>> -  i = 0;
>>>    while (ISWHITE (s[i]))
>>>      ++i;
>>
>> Instead, please move the declaration "down".
> hmm, not sure what you mean here - moving it down is bad since var
> declarations in the middle of the code are not allowed, although
> accepted by gcc.

stmt-after-decl is part of C99, now a 10-year-old standard.

> In this case its no problem, but if a while later
> someone adds code before the declaration then it clashes with non-gcc
> compilers. What's bad with initializing a var with the declaration?
> That's valid for all compilers AFAIK. Anyway, since its not needed I
> left this part out from my new patch - was just only a suggestion to
> save a line.

This is what I meant:
...
-  i = 0;
+  size_t i = 0;
   while (ISWHITE (s[i]))
     ++i;

There is already code in coreutils that requires C99 decl-after-stmt,
so even if someone added a statement before it, that would be fine.

>> That allows two or more white-space bytes.
>> Let's restrict it to just 0 or 1 space (not white-space) byte.
> ok.
>
>>> +      if (strncmp (s + j, "(", 1) == 0)
>>
>> This would be better as:
>>
>>          if (s[j] == '(')
> ok.
>
> here's 2nd trial inline (and also attached) which looks even more simple

No need to include a patch twice.

> thanks to your comments:
>
> --- src/md5sum.c.orig   2009-09-01 13:01:16.000000000 +0200
> +++ src/md5sum.c        2009-10-03 20:32:28.000000000 +0200
> @@ -263,11 +263,13 @@
>    algo_name_len = strlen (DIGEST_TYPE_STRING);
>    if (strncmp (s + i, DIGEST_TYPE_STRING, algo_name_len) == 0)
>      {
> -      if (strncmp (s + i + algo_name_len, " (", 2) == 0)
> +      if (s[i + algo_name_len] == ' ')
> +        ++i;
> +      if (s[i + algo_name_len] == '(')
>          {
>            *binary = 0;
> -          return bsd_split_3 (s +      i + algo_name_len + 2,
> -                              s_len - (i + algo_name_len + 2),
> +          return bsd_split_3 (s +      i + algo_name_len + 1,
> +                              s_len - (i + algo_name_len + 1),
>                                hex_digest, file_name);
>          }
>      }
>
>> And a test, please.
> I guess you mean something like that (also attached)?
>
...
>
> does this work for you?

Right idea.  I haven't tried them.

> Sorry, but I've not yet figured out how I could
> run these tests - a 'make tests' didnt work (therefore wrote above

"make check" runs most tests.

Use this

  make check -C tests TESTS=misc/md5sum VERBOSE=yes

to run just the one you changed.

> blindly assumed)- can you please give me a quick hint before I read me
> dead? Thanks. Once I know I create patches for the sha*sum tests too.
> Also, is it possible to run single tests?