Re: [Minios-devel] [UNIKRAFT PATCH 3/4] lib/nolibc: fix some of checkpatch issues for sscanf

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

Florian Schmidt <florian@xxxxxxxxx> writes:

> I would've also fixed the following:
>
> WARNING: break is not useful after a goto or return
> #156: FILE: lib/nolibc/sscanf.c:156:
> +                     goto doswitch;
> +                     break;
I guess this is left here because of the consistency of the
switch-case. To indicate that the case is over, and there is no "pass
through". The construction is weird, I agree. I believe there is a way
to get rid of this goto, but I'd rather not touch this code, because I
can not do a proper testing.

>
>
> But at that point, we get into personal preference territory.
>
> On 07/27/2018 05:29 PM, Yuri Volchkov wrote:
>> Signed-off-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
>
> Reviewed-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
>
>> ---
>>   lib/nolibc/sscanf.c | 37 ++++++++++++++++++++++---------------
>>   1 file changed, 22 insertions(+), 15 deletions(-)
>> 
>> diff --git a/lib/nolibc/sscanf.c b/lib/nolibc/sscanf.c
>> index 149bfbd..5e016fa 100644
>> --- a/lib/nolibc/sscanf.c
>> +++ b/lib/nolibc/sscanf.c
>> @@ -48,7 +48,7 @@ __FBSDID("$FreeBSD$");
>>    */
>>   #include <machine/stdarg.h>
>>   
>> -#define     BUF             32      /* Maximum length of numeric string. */
>> +#define     BUF             32      /* Maximum length of numeric string. */
>>   
>>   /*
>>    * Flags used during conversion.
>> @@ -105,7 +105,7 @@ __sccl(char *tab, const u_char *fmt)
>>   
>>      /* XXX: Will not work if sizeof(tab*) > sizeof(char) */
>>      for (n = 0; n < 256; n++)
>> -                 tab[n] = v;        /* memset(tab, v, 256) */
>> +            tab[n] = v;     /* memset(tab, v, 256) */
>>   
>>      if (c == 0)
>>              return (fmt - 1);/* format ended before closing ] */
>> @@ -154,7 +154,7 @@ doswitch:
>>                      fmt++;
>>                      /* fill in the range */
>>                      do {
>> -                        tab[++c] = v;
>> +                            tab[++c] = v;
>>                      } while (c < n);
>>                      c = n;
>>                      /*
>> @@ -196,11 +196,13 @@ vsscanf(const char *inp, char const *fmt0, va_list ap)
>>      char buf[BUF];          /* buffer for numeric conversions */
>>   
>>      /* `basefix' is used to avoid `if' tests in the integer scanner */
>> -    static short basefix[17] =
>> -            { 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
>> +    static short basefix[17] = {
>> +                    10, 1, 2, 3, 4, 5, 6, 7, 8, 9,
>> +                    10, 11, 12, 13, 14, 15, 16
>> +            };
>>   
>>      inr = strlen(inp);
>> -    
>> +
>>      nassigned = 0;
>>      nconversions = 0;
>>      nread = 0;
>> @@ -239,7 +241,7 @@ literal:
>>                      flags |= SUPPRESS;
>>                      goto again;
>>              case 'l':
>> -                    if (flags & LONG){
>> +                    if (flags & LONG) {
>>                              flags &= ~LONG;
>>                              flags |= QUAD;
>>                      } else {
>> @@ -250,7 +252,7 @@ literal:
>>                      flags |= QUAD;
>>                      goto again;
>>              case 'h':
>> -                    if (flags & SHORT){
>> +                    if (flags & SHORT) {
>>                              flags &= ~SHORT;
>>                              flags |= SHORTSHORT;
>>                      } else {
>> @@ -352,7 +354,7 @@ literal:
>>                              nread++;
>>                              if (--inr > 0)
>>                                      inp++;
>> -                            else
>> +                            else
>>                                      goto input_failure;
>>                      }
>>                      /*
>> @@ -373,6 +375,7 @@ literal:
>>                              width = 1;
>>                      if (flags & SUPPRESS) {
>>                              size_t sum = 0;
>> +
>>                              for (;;) {
>>                                      if ((n = inr) < width) {
>>                                              sum += n;
>> @@ -510,9 +513,13 @@ literal:
>>                                              flags |= PFXOK;
>>                                      }
>>                                      if (flags & NZDIGITS)
>> -                                        flags &= ~(SIGNOK|NZDIGITS|NDIGITS);
>> +                                            flags &= ~(SIGNOK |
>> +                                                       NZDIGITS |
>> +                                                       NDIGITS);
>>                                      else
>> -                                        flags &= ~(SIGNOK|PFXOK|NDIGITS);
>> +                                            flags &= ~(SIGNOK |
>> +                                                       PFXOK |
>> +                                                       NDIGITS);
>>                                      goto ok;
>>   
>>                              /* 1 through 7 always legal */
>> @@ -564,14 +571,14 @@ literal:
>>                               * for a number.  Stop accumulating digits.
>>                               */
>>                              break;
>> -            ok:
>> +ok:
>>                              /*
>>                               * c is legal: store it and look at the next.
>>                               */
>>                              *p++ = c;
>>                              if (--inr > 0)
>>                                      inp++;
>> -                            else
>> +                            else
>>                                      break;          /* end of input */
>>                      }
>>                      /*
>> @@ -630,9 +637,9 @@ sscanf(const char *ibuf, const char *fmt, ...)
>>   {
>>      va_list ap;
>>      int ret;
>> -    
>> +
>>      va_start(ap, fmt);
>>      ret = vsscanf(ibuf, fmt, ap);
>>      va_end(ap);
>> -    return(ret);
>> +    return ret;
>>   }
>> 
>
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel

-- 
Yuri Volchkov
Software Specialist

NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel