linux-kernel - Re: [PATCH] lib/string_helpers: add linux/string.h for strlen()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211006063029.owhu5hjtaivib5d5@ldmartin-desk2>
Date:   Tue, 5 Oct 2021 23:30:29 -0700
From:   Lucas De Marchi <lucas.demarchi@...el.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH] lib/string_helpers: add linux/string.h for strlen()

On Wed, Oct 06, 2021 at 08:57:27AM +0300, Andy Shevchenko wrote:
>On Wednesday, October 6, 2021, Lucas De Marchi <lucas.demarchi@...el.com>
>wrote:
>
>> linux/string_helpers.h uses strlen(), so include the correpondent
>> header. Otherwise we get a compilation error if it's not also included
>> by whoever included the helper.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@...el.com>
>> ---
>>  include/linux/string_helpers.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/include/linux/string_helpers.h b/include/linux/string_
>> helpers.h
>> index 68189c4a2eb1..4ba39e1403b2 100644
>> --- a/include/linux/string_helpers.h
>> +++ b/include/linux/string_helpers.h
>> @@ -4,6 +4,7 @@
>>
>>  #include <linux/bits.h>
>>  #include <linux/ctype.h>
>> +#include <linux/string.h>
>>  #include <linux/types.h>
>
>
>I’m afraid this potentially can add into header dependencies hell. What
>about moving the user to the C file?

I can do that, but I don't see the problem here... afaics it has been like this
for 7 years, since commit c8250381c827 ("lib / string_helpers: introduce string_escape_mem()"),
and the only way it was never borken is because
linux/string.h is already being indirectly included from other headers.
So just adding it here is harmless.

I reproduced this while following the normal header order in i915 and
adding linux/string_helpers.h like this:


diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 309d74fd86ce..1dfc01617258 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -3,6 +3,8 @@
   * Copyright © 2020 Intel Corporation
   */
  
+#include <linux/string_helpers.h>
+
  #include <drm/drm_debugfs.h>
  #include <drm/drm_fourcc.h>
  

Note that this became the first header included, producing the following
error:

  make -j$(nproc) drivers/gpu/drm/i915/display/intel_display_debugfs.o
   DESCEND objtool
   CALL    scripts/atomic/check-atomics.sh
   CALL    scripts/checksyscalls.sh
   CC [M]  drivers/gpu/drm/i915/display/intel_display_debugfs.o
In file included from drivers/gpu/drm/i915/display/intel_display_debugfs.c:6:
./include/linux/string_helpers.h: In function ‘string_escape_str’:
./include/linux/string_helpers.h:75:32: error: implicit declaration of function ‘strlen’ [-Werror=implicit-function-declaration]
    75 |  return string_escape_mem(src, strlen(src), dst, sz, flags, only);
       |                                ^~~~~~
./include/linux/string_helpers.h:75:32: error: incompatible implicit declaration of built-in function ‘strlen’ [-Werror]
./include/linux/string_helpers.h:7:1: note: include ‘<string.h>’ or provide a declaration of ‘strlen’
     6 | #include <linux/ctype.h>
   +++ |+#include <string.h>
     7 | #include <linux/types.h>
cc1: all warnings being treated as errors


Anyway, if it's preferable to move these functions out of line, I can do
so.

thanks
Lucas De Marchi

>
>
>>
>>  struct file;
>> --
>> 2.33.0
>>
>>
>
>-- 
>With Best Regards,
>Andy Shevchenko