On Friday 05 January 2018 14:48:39 [email protected] wrote:
> 
> 
> > -----Original Message-----
> > From: [email protected] [mailto:platform-driver-x86-
> > [email protected]] On Behalf Of Pali Rohár
> > Sent: Friday, January 5, 2018 8:44 AM
> > To: Limonciello, Mario <[email protected]>
> > Cc: [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; 
> > [email protected];
> > [email protected]; [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: Re: [PATCH v10 12/15] platform/x86: dell-smbios: Add filtering 
> > support
> > 
> > On Friday 05 January 2018 14:32:54 [email protected] wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: [email protected] 
> > > > [mailto:platform-driver-x86-
> > > > [email protected]] On Behalf Of Pali Rohár
> > > > Sent: Friday, January 5, 2018 5:13 AM
> > > > To: Limonciello, Mario <[email protected]>
> > > > Cc: [email protected]; Andy Shevchenko <[email protected]>;
> > > > LKML <[email protected]>; 
> > > > [email protected];
> > Andy
> > > > Lutomirski <[email protected]>; [email protected]; [email protected];
> > > > [email protected]; [email protected]; Greg KH <[email protected]>; Alan Cox
> > > > <[email protected]>
> > > > Subject: Re: [PATCH v10 12/15] platform/x86: dell-smbios: Add filtering 
> > > > support
> > > >
> > > > I know that this patch is already applied and merged, but I spotted this
> > > > problem:
> > > >
> > > > On Thursday 19 October 2017 12:50:15 Mario Limonciello wrote:
> > > > > +/* calls that are explicitly blacklisted */
> > > > > +static struct smbios_call call_blacklist[] = {
> > > > > +     {0x0000, 01, 07}, /* manufacturing use */
> > > > > +     {0x0000, 06, 05}, /* manufacturing use */
> > > > > +     {0x0000, 11, 03}, /* write once */
> > > > > +     {0x0000, 11, 07}, /* write once */
> > > >
> > > > Numbers prefixed by zero means that they are in octal notation, right?
> > > Is that how the kernel interprets an integer prefix by zero?
> > 
> > No, this is how C language define it. See e.g. C11 standard, section
> > 6.4.4.1 Integer constants:
> > 
> > decimal-constant:
> >     nonzero-digit
> >     decimal-constant digit
> > 
> > octal-constant:
> >     0
> >     octal-constant octal-digit
> > 
> > So in C decimal number cannot start with digit zero.
> > 
> > I think the place where octal numbers are used are in permissions (0777)
> > 
> > > I prefixed by zero for readability, they're supposed to be decimal.
> > >
> > > > This can lead to misunderstanding, confusion or problems in future...
> > > >
> > > > Can we have all numbers either in hexadecimal or decimal notation?
> > >
> > > Could you elaborate more why this is problematic the way it is?
> > 
> > Currently it is not problem as 7 is same number in octal (07) and
> > decimal (7). representation. But e.g. octal 077 is 63 in decimal.
> > 
> > > Are you meaning you would rather see this?
> > >   {0x0000, 1, 7}, /* manufacturing use */
> > >   {0x0000, 6, 5}, /* manufacturing use */
> > >   {0x0000, 11, 3}, /* write once */
> > >   {0x0000, 11, 7}, /* write once */
> > 
> > Yes, this is better. If you need to achieve alignment then use spaces.
> > Really, not leading zeros.
> > 
> > > That seems less readable to me but should interpret the same way.
> > 
> > Example:
> > 
> > {0x000, 077, 7},
> > {0x000, 007, 7},
> > 
> > is **not** same as
> > 
> > {0x000,  77, 7},
> > {0x000,   7, 7},
> > 
> > As first number in first section is (decimal) 63, not (decimal) 77.
> > 
> > > Perhaps it would be better if you submit a patch with what is clearer to
> > > you.
> > >
> > > >
> > > > > +     {0x0000, 11, 11}, /* write once */
> > > > > +     {0x0000, 19, -1}, /* diagnostics */
> > > > > +     /* handled by kernel: dell-laptop */
> > > > > +     {0x0000, CLASS_INFO, SELECT_RFKILL},
> > > > > +     {0x0000, CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT},
> > > > > +};
> > > >
> > > > --
> > > > Pali Rohár
> > > > [email protected]
> > 
> > --
> > Pali Rohár
> > [email protected]
> 
> Thanks very much for sharing.  I wasn't aware of this.  I'll send a patch.

Hi! Do you have a patch for it?

-- 
Pali Rohár
[email protected]

Attachment: signature.asc
Description: PGP signature

Reply via email to