linux-kernel - Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default source directory

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-prev] [day] [month] [year] [list]

Message-ID: <20210208195439.GA1097868@ubuntu-m3-large-x86>
Date:   Mon, 8 Feb 2021 12:54:39 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Stephen Zhang <stephenzhangzsd@...il.com>
Cc:     ndesaulniers@...gle.com, natechancellor@...il.com,
        clang-built-linux@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] clang_tools:gen_compile_commands: Change the default
 source directory

On Mon, Feb 08, 2021 at 07:28:57PM +0800, Stephen Zhang wrote:
> The default source directory is set equal to build directory which
> specified by "-d".But it is designed to be set to the current working
> directoy by default, as the help messge says.It makes a differece when
> source directory and build directory are in separted directorys.
> 
> Signed-off-by: Stephen Zhang <stephenzhangzsd@...il.com>

I don't think this patch makes much sense unless I am misunderstanding
the description of the problem. The entire point of this script is to
parse the .cmd files that kbuild generates and those are only present
in the build directory, not the source directory, so we should never be
looking in there, unless args.directory is its default value, which is
the way the script is currently written. Your patch would appear to
either make use do way more searching than necessary (if the build
folder is within the source folder) or miss it altogether (if the build
folder is outside the source folder).

I think the help message probably needs to be updated to be a little
clearer:

diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
index 19963708bcf8..cf36d73dd3bf 100755
--- a/scripts/clang-tools/gen_compile_commands.py
+++ b/scripts/clang-tools/gen_compile_commands.py
@@ -55,7 +55,7 @@ def parse_arguments():
 
     paths_help = ('directories to search or files to parse '
                   '(files should be *.o, *.a, or modules.order). '
-                  'If nothing is specified, the current directory is searched')
+                  'If nothing is specified, the output directory is searched')
     parser.add_argument('paths', type=str, nargs='*', help=paths_help)
 
     args = parser.parse_args()

Let me know if this makes sense or if I am completely off base :)

Cheers,
Nathan

> ---
>  scripts/clang-tools/gen_compile_commands.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
> index 1996370..add9e47 100755
> --- a/scripts/clang-tools/gen_compile_commands.py
> +++ b/scripts/clang-tools/gen_compile_commands.py
> @@ -64,7 +64,7 @@ def parse_arguments():
>              os.path.abspath(args.directory),
>              args.output,
>              args.ar,
> -            args.paths if len(args.paths) > 0 else [args.directory])
> +            args.paths if len(args.paths) > 0 else [os.getcwd()])
>  
>  
>  def cmdfiles_in_dir(directory):
> -- 
> 1.8.3.1
>