lkml.org 
[lkml]   [2018]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH AUTOSEL for 4.15 142/189] firmware: dmi_scan: Fix handling of empty DMI strings
    Hi Parag,

    On Sun, 8 Apr 2018 21:21:19 -0400, Parag Warudkar wrote:
    > On Sun, Apr 8, 2018 at 8:18 PM, Sasha Levin <Alexander.Levin@microsoft.com>
    > wrote:
    >
    > > From: Jean Delvare <jdelvare@suse.de>
    > >
    > > [ Upstream commit a7770ae194569e96a93c48aceb304edded9cc648 ]
    > >
    > > [snip]
    > >
    >
    >
    > > * Strings starting with 8 spaces are all considered empty, even if
    > > non-space characters follow (sounds like a weird thing to do, but
    > > I have actually seen occurrences of this in DMI tables before.)
    > >
    >
    > Unless I am misreading the patch we will now allocate memory for
    > len(spaces)+len(string) for this case.

    Correct.

    > Have you seen BIOSes with lots of <space>string occurrences?

    A fair number, yes, but most of them were thankfully not in strings we
    are saving. Also note that most of them only have 1 or 2 leading
    spaces, not 8+, so this commit makes no difference for them.

    > If so what's the point of allocating memory for the leading spaces?

    Presented like that, it sounds like we are silly. But look at things
    the other way around: what's the point of storing these leading spaces
    in the DMI table in the first place? Whenever this happens, the
    hardware manufacturer is to blame, not us.

    As much as possible, we should stick to the specification and assume
    the other party does as well. Stripping the leading spaces would be
    trivial, but hiding their mistakes does not help hardware manufacturers
    in the long run anyway. If they can't see that it's wrong, it's harder
    for them to fix it.

    > IOW, what happens if we strip leading space before allocating?

    At boot time, we save a few bytes of memory, on the affected systems
    only. No code cost, but possibly some confusion as to why we strip
    leading spaces (which are rare) but not trailing spaces (which in my
    experience are more frequent.)

    At run time, the exact matching of the affected DMI strings would be
    less error-prone (although strings having leading spaces are likely to
    also have trailing spaces, annihilating the benefits.) Non-exact
    matching would be marginally faster, again only for the affected
    systems.

    As a conclusion, it's doable, but the benefit is very small and limited
    to a few broken systems, and it has the downside of not discouraging
    low-quality tables, so my position is that it's not worth it and not
    desirable.

    --
    Jean Delvare
    SUSE L3 Support

    \
     
     \ /
      Last update: 2018-04-09 14:48    [W:4.523 / U:1.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site