Victor Bravo <1905@xxxxxxxxxx> writes: > On Mon, May 06, 2019 at 11:42:06AM +0300, Kalle Valo wrote: >> Hans de Goede <hdegoede@xxxxxxxxxx> writes: >> >> >> @@ -99,6 +107,15 @@ static const struct dmi_system_id dmi_platform_data[] = { >> >> {} >> >> }; >> >> +void brcmf_dmi_sanitize(char *dst, const unsigned char *allowed, >> >> char safe) >> >> +{ >> >> + while (*dst) { >> >> + if ((*dst < 0) || !(allowed[*dst / 8] & (1 << (*dst % 8)))) >> > >> > At a first look I have no clue what this code is doing and I honestly do not feel >> > like figuring it out, this is clever, but IMHO not readable. >> > >> > Please just write this as if (*dst < 0x21 || (*dst > foo && < bar) || etc, >> > so that a human can actually see in one look what the code is doing. >> >> Is there an existing function for sanitising filenames so that we don't >> need to reinvent the wheel, maybe something like isalnum()? > > I would definitely prefer to use existing function, but I didn't find > any suitable one. Suggestions are welcome. I didn't find anything either, but hopefully someone knows. > As for implementation details, the one I posted was optimized for both > speed and size, and at least in my opinion this bit array driven > parametric implementation is exactly what is needed here (using a string > of allowed characters with strchr-style lookups would bring much worse > complexity, and checking the characters using series of hardcoded if > conditions could quickly grow to more than those 16 bytes used by the > array). But is this really something which should be optimised? This is driver initialisation, not in some hot path, right? Can you even measure the difference? -- Kalle Valo