Re: [Minios-devel] [UNIKRAFT PATCH 4/4] lib/nolibc: adapt sscanf code for Unikraft

[Florian Schmidt <florian@xxxxxxxxx>]

On 07/27/2018 05:29 PM, Yuri Volchkov wrote:
> 1) Use the right includes
> 2) (u_)quad_t => (u)int64_t
> 3) u_char => unsigned char
> 4) strto(u)q => strto(u)ll
> 5) bcopy => memmove
> 6) fix warnings generated by modern gcc (8.1.1)
That's the explicit casts to ccfntype and int that you added?

> diff --git a/lib/nolibc/include/stdio.h b/lib/nolibc/include/stdio.h
> index 073b132..6d5652f 100644
> --- a/lib/nolibc/include/stdio.h
> +++ b/lib/nolibc/include/stdio.h
> @@ -64,6 +64,9 @@ int   fflush(FILE *fp);
>   int vprintf(const char *fmt, va_list ap);
>   int  printf(const char *fmt, ...)                           __printf(1, 2);
>   
> +int vsscanf(const char *str, const char *fmt, va_list ap);
> +int  sscanf(const char *str, const char *fmt, ...) __scanf(2, 3);
I would align the __scanf with the preceding __printf statement for 
consistency.
> @@ -271,32 +261,32 @@ literal:
>                  */
>                 case 'd':
>                         c = CT_INT;
> -                       ccfn = (ccfntype)strtoq;
> +                       ccfn = (ccfntype)strtoll;
>                         base = 10;
>                         break;
>   
>   		case 'i':
>                         c = CT_INT;
> -                       ccfn = (ccfntype)strtoq;
> +                       ccfn = (ccfntype)strtoll;
>                         base = 0;
>                         break;
>   
>   		case 'o':
>                         c = CT_INT;
> -                       ccfn = strtouq;
> +                       ccfn = (ccfntype) strtoull;
>                         base = 8;
>                         break;
Whenever you added your explicit cast, you put a space there; when you 
only changed the function name, you left it without a space. Could you 
make this consistent? (I personally prefer the version without a space 
for casts, but I don't think we have a coding style rule for it. Just 
consistent inside the same switch statement would be nice.)
This applies to the ones, below, too:

>   
>   		case 'u':
>                         c = CT_INT;
> -                       ccfn = strtouq;
> +                       ccfn = (ccfntype) strtoull;
>                         base = 10;
>                         break;
>   
>   		case 'x':
>                         flags |= PFXOK; /* enable 0x prefixing */
>                         c = CT_INT;
> -                       ccfn = strtouq;
> +                       ccfn = (ccfntype) strtoull;
>                         base = 16;
>                         break;
>   
> @@ -318,7 +308,7 @@ literal:
>                 case 'p':       /* pointer format is like hex */
>                         flags |= POINTER | PFXOK;
>                         c = CT_INT;
> -                       ccfn = strtouq;
> +                       ccfn = (ccfntype) strtoull;
>                         base = 16;
>                         break;
>   


> diff --git a/lib/nolibc/stdio.c b/lib/nolibc/stdio.c
> index 7e3d368..3a32907 100644
> --- a/lib/nolibc/stdio.c
> +++ b/lib/nolibc/stdio.c
> @@ -289,6 +289,7 @@ reswitch:
>                         goto handle_nosign;
>                 case 'X':
>                         upper = 1;
> +                       /* Fall through */
>                 case 'x':
>                         base = 16;
>                         goto handle_nosign;
That last bit is unrelated to the rest of the patch?


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