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.le...@microsoft.com> > wrote: > > > From: Jean Delvare <jdelv...@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