Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1630058 > unrolled thread

Re: [PATCH] FS-Cache: print hexadecimal value for special cookies type

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] FS-Cache: print hexadecimal value for special cookies type Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-04-24 23:50 +0200
    Re: [PATCH] FS-Cache: print hexadecimal value for special cookies  type Jérémy Lefaure <jeremy.lefaure@lse.epita.fr> - 2017-04-25 23:40 +0200
      Re: [PATCH] FS-Cache: print hexadecimal value for special cookies type Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-04-26 00:10 +0200

#1630058 — Re: [PATCH] FS-Cache: print hexadecimal value for special cookies type

On Sat, Apr 22, 2017 at 2:09 AM, Jérémy Lefaure
<jeremy.lefaure@lse.epita.fr> wrote:
> On Fri, 21 Apr 2017 22:41:54 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>> On Wed, Apr 19, 2017 at 11:38 PM, Jérémy Lefaure
>> <jeremy.lefaure@lse.epita.fr> wrote:
>> > When building object-list.o, gcc 6 raises a warning on the sprintf call
>> > in fscache_objlist_show:
>> >
>> >   CC      fs/fscache/object-list.o
>> > fs/fscache/object-list.c: In function ‘fscache_objlist_show’:
>> > fs/fscache/object-list.c:265:19: warning: ‘sprintf’ may write a
>> > terminating nul past the end of the destination [-Wformat-overflow=]
>> >     sprintf(_type, "%02u", cookie->def->type);
>> >                    ^~~~~~
>> > fs/fscache/object-list.c:265:4: note: ‘sprintf’ output between 3 and 4
>> > bytes into a destination of size 3
>> >     sprintf(_type, "%02u", cookie->def->type);
>> >     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >
>> > Moreover, the documentation says that we should have an hex value for
>> > special cookies (see Documentation/filesystems/caching/fscache.txt).
>> >
>> > Printing hexadecimal value for special cookies fixes the overflow
>> > warning and complies with the documentation.
>>
>> If this is used by some user space tool the safest fix is to print in
>> BCD and fix documentation.
>>
> Are you talking about the Binary Coded Decimal format ?

Yes.

> I don't get why
> it would be better. If we change the printed format, it would break
> user space tool anyway, right ?

If userspace tool is bound to the format it might go crazy about hex
digits in the cookie.

Is that cookie important somehow? (for example, user space might
request something based on it?) It might be a case when you got let
say '75' and then ask for 117 (0x75), however real value is 75.

So, if it's okay in this, perhaps paragraph explaining side effects in
commit message would be good to have/

-- 
With Best Regards,
Andy Shevchenko

[toc] | [next] | [standalone]

#1631018 — Re: [PATCH] FS-Cache: print hexadecimal value for special cookies type

On Tue, 25 Apr 2017 00:47:41 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Apr 22, 2017 at 2:09 AM, Jérémy Lefaure
> <jeremy.lefaure@lse.epita.fr> wrote:
> > On Fri, 21 Apr 2017 22:41:54 +0300
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:  
> >> On Wed, Apr 19, 2017 at 11:38 PM, Jérémy Lefaure
> >> <jeremy.lefaure@lse.epita.fr> wrote:  
> >> > When building object-list.o, gcc 6 raises a warning on the sprintf call
> >> > in fscache_objlist_show:
> >> >
> >> >   CC      fs/fscache/object-list.o
> >> > fs/fscache/object-list.c: In function ‘fscache_objlist_show’:
> >> > fs/fscache/object-list.c:265:19: warning: ‘sprintf’ may write a
> >> > terminating nul past the end of the destination [-Wformat-overflow=]
> >> >     sprintf(_type, "%02u", cookie->def->type);
> >> >                    ^~~~~~
> >> > fs/fscache/object-list.c:265:4: note: ‘sprintf’ output between 3 and 4
> >> > bytes into a destination of size 3
> >> >     sprintf(_type, "%02u", cookie->def->type);
> >> >     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >
> >> > Moreover, the documentation says that we should have an hex value for
> >> > special cookies (see Documentation/filesystems/caching/fscache.txt).
> >> >
> >> > Printing hexadecimal value for special cookies fixes the overflow
> >> > warning and complies with the documentation.  
> >>
> >> If this is used by some user space tool the safest fix is to print in
> >> BCD and fix documentation.
> >>  
> > Are you talking about the Binary Coded Decimal format ?  
> 
> Yes.
> 
> > I don't get why
> > it would be better. If we change the printed format, it would break
> > user space tool anyway, right ?  
> 
> If userspace tool is bound to the format it might go crazy about hex
> digits in the cookie.
> 
> Is that cookie important somehow? (for example, user space might
> request something based on it?) It might be a case when you got let
> say '75' and then ask for 117 (0x75), however real value is 75.
> 
Ok I get it. But changing the format can break the userland tool
anyway, can't it ? For example if a userland tool expects the value 75,
it won't read it properpy in BCD.

> So, if it's okay in this, perhaps paragraph explaining side effects in
> commit message would be good to have/
> 
Maybe the best fix would be to increase the size of the _type buffer to
4 so the unsigned byte can fit in it. And change the documentation to
decimal value. What do you think about this solution ?

Thanks,

[toc] | [prev] | [next] | [standalone]

#1631029

On Wed, Apr 26, 2017 at 12:37 AM, Jérémy Lefaure
<jeremy.lefaure@lse.epita.fr> wrote:
> On Tue, 25 Apr 2017 00:47:41 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> Ok I get it. But changing the format can break the userland tool
> anyway, can't it ? For example if a userland tool expects the value 75,
> it won't read it properpy in BCD.

Maybe.

>> So, if it's okay in this, perhaps paragraph explaining side effects in
>> commit message would be good to have/
>>
> Maybe the best fix would be to increase the size of the _type buffer to
> 4 so the unsigned byte can fit in it. And change the documentation to
> decimal value. What do you think about this solution ?

Yes, it would work.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web