[PATCH] docs: add extra integer types to printk-formats

[PATCH] docs: add extra integer types to printk-formats

[Louis Taylor]

A few commonly used integer types were absent from this table, so add
them.

Link: https://github.com/ClangBuiltLinux/linux/issues/378
Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Louis Taylor <louis@kragniz.eu>
---
 Documentation/core-api/printk-formats.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index a7fae4538946..6f08b1b6240a 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -19,8 +19,16 @@ Integer types
 		unsigned long		%lu or %lx
 		long long		%lld or %llx
 		unsigned long long	%llu or %llx
+		short int		%hd or %hx
+		unsigned short int	%hu or %hx
+		char			%hhd or %hhx
+		unsigned char		%hhu or %hhx
 		size_t			%zu or %zx
 		ssize_t			%zd or %zx
+		s8			%hhd or %hhx
+		u8			%hhu or %hhx
+		s16			%hd or %hx
+		u16			%hu or %hx
 		s32			%d or %x
 		u32			%u or %x
 		s64			%lld or %llx
-- 
2.20.1

Re: [PATCH] docs: add extra integer types to printk-formats

[Andy Shevchenko]

On Sun, Mar 03, 2019 at 12:36:47PM +0000, Louis Taylor wrote:
> A few commonly used integer types were absent from this table, so add
> them.

I'm not against the patch, but isn't obvious by reading POSIX and / or man
printf(3)?

> Link: https://github.com/ClangBuiltLinux/linux/issues/378

Thanks for the link.

It's interesting to see how compiler type mindset applies to fixing drivers and
hardware related code...

> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Louis Taylor <louis@kragniz.eu>
> ---
>  Documentation/core-api/printk-formats.rst | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index a7fae4538946..6f08b1b6240a 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -19,8 +19,16 @@ Integer types
>  		unsigned long		%lu or %lx
>  		long long		%lld or %llx
>  		unsigned long long	%llu or %llx
> +		short int		%hd or %hx
> +		unsigned short int	%hu or %hx
> +		char			%hhd or %hhx
> +		unsigned char		%hhu or %hhx
>  		size_t			%zu or %zx
>  		ssize_t			%zd or %zx
> +		s8			%hhd or %hhx
> +		u8			%hhu or %hhx
> +		s16			%hd or %hx
> +		u16			%hu or %hx
>  		s32			%d or %x
>  		u32			%u or %x
>  		s64			%lld or %llx
> -- 
> 2.20.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] docs: add extra integer types to printk-formats

[Nick Desaulniers]

On Sun, Mar 3, 2019 at 12:10 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Sun, Mar 03, 2019 at 12:36:47PM +0000, Louis Taylor wrote:
> > A few commonly used integer types were absent from this table, so add
> > them.
>
> I'm not against the patch, but isn't obvious by reading POSIX and / or man
> printf(3)?

You'd think; but based on the sheer number of -Wformat warnings
(~450), I'm not so sure.  At least with this patch they're "above the
fold."

The kernel also has its own format flag extensions, and does not
implement %n (for good reason), so it's better to be explicit than
imply posix or glibc compat.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] docs: add extra integer types to printk-formats

[Joe Perches]

On Mon, 2019-03-04 at 09:59 -0800, Nick Desaulniers wrote:
> On Sun, Mar 3, 2019 at 12:10 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Sun, Mar 03, 2019 at 12:36:47PM +0000, Louis Taylor wrote:
> > > A few commonly used integer types were absent from this table, so add
> > > them.
> > 
> > I'm not against the patch, but isn't obvious by reading POSIX and / or man
> > printf(3)?
> 
> You'd think; but based on the sheer number of -Wformat warnings
> (~450), I'm not so sure.

<shrug>  software defects are always present.

Many of the -Wformat warnings are bogus too.

There's nothing wrong with using %x for a unsigned int
of less than long size. (u8/u16)

>   At least with this patch they're "above the
> fold."

I'd personally go with
"posix plus kernel specific deletions and extensions"

> The kernel also has its own format flag extensions, and does not
> implement %n (for good reason), so it's better to be explicit than
> imply posix or glibc compat.

%i is also supported and used a few hundred times

Re: [PATCH] docs: add extra integer types to printk-formats

[Matthew Wilcox]

On Tue, Mar 05, 2019 at 11:59:30AM -0800, Joe Perches wrote:
> Many of the -Wformat warnings are bogus too.
> 
> There's nothing wrong with using %x for a unsigned int
> of less than long size. (u8/u16)

I believe you to be incorrect.

6.5.2.2 Function calls

7 If the expression that denotes the called function has a type that does
  include a prototype, the arguments are implicitly converted, as if by
  assignment, to the types of the corresponding parameters, taking the
  type of each parameter to be the unqualified version of its declared
  type. The ellipsis notation in a function prototype declarator causes
  argument type conversion to stop after the last declared parameter. The
  default argument promotions are performed on trailing arguments.

I could define a calling convention for my CPU which says to pack u8s
and u16s as tightly as possible in registers (or on the stack), rather
than the prevailing calling convention of having each argument take up
at least one register-sized slot.

Re: [PATCH] docs: add extra integer types to printk-formats

[Matthew Wilcox]

On Tue, Mar 05, 2019 at 12:07:25PM -0800, Matthew Wilcox wrote:
> On Tue, Mar 05, 2019 at 11:59:30AM -0800, Joe Perches wrote:
> > Many of the -Wformat warnings are bogus too.
> > 
> > There's nothing wrong with using %x for a unsigned int
> > of less than long size. (u8/u16)
> 
> I believe you to be incorrect.

No, you're right, I got tangled up in the spec language and stopped
reading too soon.

> 6.5.2.2 Function calls
> 
> 7 If the expression that denotes the called function has a type that does
>   include a prototype, the arguments are implicitly converted, as if by
>   assignment, to the types of the corresponding parameters, taking the
>   type of each parameter to be the unqualified version of its declared
>   type. The ellipsis notation in a function prototype declarator causes
>   argument type conversion to stop after the last declared parameter. The
>   default argument promotions are performed on trailing arguments.

... the default argument promotions performed on the trailing arguments
promote short/char to int.  So you were right.

> I could define a calling convention for my CPU which says to pack u8s
> and u16s as tightly as possible in registers (or on the stack), rather
> than the prevailing calling convention of having each argument take up
> at least one register-sized slot.

... and that is just wrong; it's not allowed.

Re: [PATCH] docs: add extra integer types to printk-formats

[Joe Perches]

On Tue, 2019-03-05 at 12:07 -0800, Matthew Wilcox wrote:
> On Tue, Mar 05, 2019 at 11:59:30AM -0800, Joe Perches wrote:
> > Many of the -Wformat warnings are bogus too.
> > 
> > There's nothing wrong with using %x for a unsigned int
> > of less than long size. (u8/u16)
> 
> I believe you to be incorrect.
> 
> 6.5.2.2 Function calls
> 
> 7 If the expression that denotes the called function has a type that does
>   include a prototype, the arguments are implicitly converted, as if by
>   assignment, to the types of the corresponding parameters, taking the
>   type of each parameter to be the unqualified version of its declared
>   type. The ellipsis notation in a function prototype declarator causes
>   argument type conversion to stop after the last declared parameter. The
>   default argument promotions are performed on trailing arguments.

Look at default argument promotions for varargs functions.
(integer promotions)

> I could define a calling convention for my CPU which says to pack u8s
> and u16s as tightly as possible in registers (or on the stack), rather
> than the prevailing calling convention of having each argument take up
> at least one register-sized slot.

Doesn't matter for varargs.  See above.

cheers, Joe

Re: [PATCH] docs: add extra integer types to printk-formats

[Randy Dunlap]

On 3/5/19 11:59 AM, Joe Perches wrote:
> On Mon, 2019-03-04 at 09:59 -0800, Nick Desaulniers wrote:
>> On Sun, Mar 3, 2019 at 12:10 PM Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>>> On Sun, Mar 03, 2019 at 12:36:47PM +0000, Louis Taylor wrote:
>>>> A few commonly used integer types were absent from this table, so add
>>>> them.
>>>
>>> I'm not against the patch, but isn't obvious by reading POSIX and / or man
>>> printf(3)?
>>
>> You'd think; but based on the sheer number of -Wformat warnings
>> (~450), I'm not so sure.
> 
> <shrug>  software defects are always present.
> 
> Many of the -Wformat warnings are bogus too.
> 
> There's nothing wrong with using %x for a unsigned int
> of less than long size. (u8/u16)
> 
>>   At least with this patch they're "above the
>> fold."
> 
> I'd personally go with
> "posix plus kernel specific deletions and extensions"

Yeah, I don't think that all of the "standard" types/formats need
to be in there.  Just the differences.

>> The kernel also has its own format flag extensions, and does not
>> implement %n (for good reason), so it's better to be explicit than
>> imply posix or glibc compat.
> 
> %i is also supported and used a few hundred times
> 
> 


-- 
~Randy

Re: [PATCH] docs: add extra integer types to printk-formats

[Petr Mladek]

On Sun 2019-03-03 12:36:47, Louis Taylor wrote:
> A few commonly used integer types were absent from this table, so add
> them.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Louis Taylor <louis@kragniz.eu>
> ---
>  Documentation/core-api/printk-formats.rst | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index a7fae4538946..6f08b1b6240a 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -19,8 +19,16 @@ Integer types
>  		unsigned long		%lu or %lx
>  		long long		%lld or %llx
>  		unsigned long long	%llu or %llx
> +		short int		%hd or %hx
> +		unsigned short int	%hu or %hx
> +		char			%hhd or %hhx
> +		unsigned char		%hhu or %hhx

These should get moved to the beginning of the table to
have all variants sorted by size.

>  		size_t			%zu or %zx
>  		ssize_t			%zd or %zx
> +		s8			%hhd or %hhx
> +		u8			%hhu or %hhx
> +		s16			%hd or %hx
> +		u16			%hu or %hx
>  		s32			%d or %x
>  		u32			%u or %x
>  		s64			%lld or %llx

Let's get it in when people are doing the clean up.

I have fixed the sorting and committed into printk.git,
branch for-5.1, see
https://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git/commit/?h=for-5.1&id=943ca6ad81e5b37bd5d22cbab5ad06dc97fc001c

Best Regards,
Petr

Re: [PATCH] docs: add extra integer types to printk-formats

[Nick Desaulniers]

On Mon, Mar 4, 2019 at 7:45 AM Petr Mladek <pmladek@suse.com> wrote:
>
> On Sun 2019-03-03 12:36:47, Louis Taylor wrote:
> > A few commonly used integer types were absent from this table, so add
> > them.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/378
> > Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> > Signed-off-by: Louis Taylor <louis@kragniz.eu>
> > ---
> >  Documentation/core-api/printk-formats.rst | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> > index a7fae4538946..6f08b1b6240a 100644
> > --- a/Documentation/core-api/printk-formats.rst
> > +++ b/Documentation/core-api/printk-formats.rst
> > @@ -19,8 +19,16 @@ Integer types
> >               unsigned long           %lu or %lx
> >               long long               %lld or %llx
> >               unsigned long long      %llu or %llx
> > +             short int               %hd or %hx
> > +             unsigned short int      %hu or %hx
> > +             char                    %hhd or %hhx
> > +             unsigned char           %hhu or %hhx
>
> These should get moved to the beginning of the table to
> have all variants sorted by size.
>
> >               size_t                  %zu or %zx
> >               ssize_t                 %zd or %zx
> > +             s8                      %hhd or %hhx
> > +             u8                      %hhu or %hhx
> > +             s16                     %hd or %hx
> > +             u16                     %hu or %hx
> >               s32                     %d or %x
> >               u32                     %u or %x
> >               s64                     %lld or %llx
>
> Let's get it in when people are doing the clean up.
>
> I have fixed the sorting and committed into printk.git,
> branch for-5.1, see
> https://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git/commit/?h=for-5.1&id=943ca6ad81e5b37bd5d22cbab5ad06dc97fc001c

Sure that's fine.  Thanks for keeping Louis as the author, and for the clean up.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

-- 
Thanks,
~Nick Desaulniers