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

[Lucas De Marchi]

On Wed, Oct 06, 2021 at 08:57:27AM +0300, Andy Shevchenko wrote:
> On Wednesday, October 6, 2021, Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> 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@xxxxxxxxx>
> > ---
> >  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