[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/createyourpersonalaccount closed https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
createyourpersonalaccount wrote: I don't know what you're talking about. I know however that you're bikeshedding. Sad. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
owenca wrote: > > Diagnostic output should go to stderr, but the informational output of > > git-clang-format is sent to stdout instead. > > What does that mean? The printing of null-terminated paths on stdout is > pretty standard on unix utilities. What's "diagnostic" and "informational"? Diagnostic messages are error/warning/informational messages. > The `--null` option is so that the paths can be safely handled by other tools > in the shell pipeline. > > > If you want to run git add in git-clang-format, we can add an option for > > that. > > No, that's limiting to the user. The right thing to do is to print the paths > with null-terminated bytes in between, so that any utility wished can be > executed. As you already admitted in https://github.com/llvm/llvm-project/pull/123926#issuecomment-2614296866 that you would only use this on `git clang-format --staged` with `git add` and didn't know about other use cases, how can you be sure it's the "right thing to do"? > Are you perhaps not aware of the issues at play here? Paths on linux/bsd/etc > can contain any character. It's impossible to parse text output containing > paths safely. You have to use some kind of format, and the approach used by > all utilities is null-separated paths. I used `find -print0 ... | xargs -0` myself before. That's why I requested that you rename `-0, --null` to `-print0`. I don't know what @mydeveloperday and @HazardyKnusperkeks think, but I wouldn't be in support of your proposed option for the reasons mentioned earlier. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
createyourpersonalaccount wrote: > Diagnostic output should go to stderr, but the informational output of > git-clang-format is sent to stdout instead. What does that mean? The printing of null-terminated paths on stdout is pretty standard on unix utilities. What's "diagnostic" and "informational"? The `--null` option is so that the paths can be safely handled by other tools in the shell pipeline. > If you want to run git add in git-clang-format, we can add an option for that. No, that's limiting to the user. The right thing to do is to print the paths with null-terminated bytes in between, so that any utility wished can be executed. Are you perhaps not aware of the issues at play here? Paths on linux/bsd/etc can contain any character. It's impossible to parse text output containing paths safely. You have to use some kind of format, and the approach used by all utilities is null-separated paths. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
owenca wrote: > > It seems that `git-clang-format` was not designed for its output to be > > consumed by another utility as it prints everything to `stdout` instead of > > `stderr`. Adding an option like`--print0` (especially just for `--staged`) > > looks odd to me. > > No, the approach of printing to `stderr` for utilities is not accurate. I've > heard of this before, even the claim that this is the original UNIX way, but > it's not. I don't know the origin of this > [cargo-culting](https://en.wikipedia.org/wiki/Cargo_cult) factoid but I > assure you that it does not hold true. (The origin I suspect may be Microsoft > with its Powershell, i.e. see > [about_Output_Streams](https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_output_streams?view=powershell-7.5)) Diagnostic output should go to stderr, but the informational output of git-clang-format is sent to stdout instead. > There is a lot of value in `--null` since it allows `git clang-format` to be > used in git hooks. Why would a Python script designed as a git subcommand not > work well with git? If you want to run `git add` in git-clang-format, we can add an option for that. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
createyourpersonalaccount wrote: @owenca ping https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -260,15 +265,13 @@ def main():
"Ignoring the following files (wrong extension, symlink, or "
"ignored by clang-format):"
)
-for filename in ignored_files:
-print("%s" % filename)
+print_filenames(ignored_files, opts.print0)
createyourpersonalaccount wrote:
@HazardyKnusperkeks Do you have an input on this? Right now it's just ignoring
`--verbose`.
https://github.com/llvm/llvm-project/pull/123926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
createyourpersonalaccount wrote: > > > From #123921, it seems that you only want the new option to work with > > > `--staged`, but should it also work with other options that may print a > > > list of filenames? > > > > > > I don't know; I only use `git clang-format` as in #123921 and I am not > > familiar with all its uses. I was hoping that you or someone other who is > > familiar could determine this. > > What worries me is conditions where more than one list of files is printed, > > e.g. unchanged + changed files. It's difficult to parse these messages from > > the shell; when a null-separated list is printed from other command line > > utilities, it has one meaning, e.g. "here's the list of files you asked > > for", whereas `git clang-format` prints other diagnostics too. > > If someone is familiar with the usages of this tool maybe they can tell me > > what is the logic that must be followed in terms of **which list** is the > > important one according to the options passed, so that only that list is > > printed with `--print0`. > > It seems that `git-clang-format` was not designed for its output to be > consumed by another utility as it prints everything to `stdout` instead of > `stderr`. Adding an option like`--print0` (especially just for `--staged`) > looks odd to me. No, the approach of printing to `stderr` for utilities is not accurate. I've heard of this before, even the claim that this is the original UNIX way, but it's not. I don't know the origin of this [cargo-culting](https://en.wikipedia.org/wiki/Cargo_cult) factoid but I assure you that it does not hold true. (The origin I suspect may be Microsoft with its Powershell, i.e. see [about_Output_Streams](https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_output_streams?view=powershell-7.5)) There is a lot of value in `--null` since it allows `git clang-format` to be used in git hooks. Why would a Python script designed as a git subcommand not work well with git? https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
owenca wrote: > > From #123921, it seems that you only want the new option to work with > > `--staged`, but should it also work with other options that may print a > > list of filenames? > > I don't know; I only use `git clang-format` as in #123921 and I am not > familiar with all its uses. I was hoping that you or someone other who is > familiar could determine this. > > What worries me is conditions where more than one list of files is printed, > e.g. unchanged + changed files. It's difficult to parse these messages from > the shell; when a null-separated list is printed from other command line > utilities, it has one meaning, e.g. "here's the list of files you asked for", > whereas `git clang-format` prints other diagnostics too. > > If someone is familiar with the usages of this tool maybe they can tell me > what is the logic that must be followed in terms of **which list** is the > important one according to the options passed, so that only that list is > printed with `--print0`. It seems that `git-clang-format` was not designed for its output to be consumed by another utility as it prints everything to `stdout` instead of `stderr`. Adding an option like`--print0` (especially just for `--staged`) looks odd to me. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/createyourpersonalaccount edited https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -260,15 +265,13 @@ def main():
"Ignoring the following files (wrong extension, symlink, or "
"ignored by clang-format):"
)
-for filename in ignored_files:
-print("%s" % filename)
+print_filenames(ignored_files, opts.print0)
createyourpersonalaccount wrote:
You have a good point, should they be mutually exclusive? I.e. if both are
specified an error message is printed. I don't think `--verbose` is meaningful
because `--null` does not have extra information to print.
https://github.com/llvm/llvm-project/pull/123926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -260,15 +265,13 @@ def main():
"Ignoring the following files (wrong extension, symlink, or "
"ignored by clang-format):"
)
-for filename in ignored_files:
-print("%s" % filename)
+print_filenames(ignored_files, opts.print0)
HazardyKnusperkeks wrote:
Is verbose and null a combination that should be supported?
https://github.com/llvm/llvm-project/pull/123926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
createyourpersonalaccount wrote: @owenca @mydeveloperday @HazardyKnusperkeks I'll ping now that it's the weekend, perhaps you have some time to look at this more; in particular now I got the `--null` option to work for `--staged`, but I'm not sure how you'd like it to work for other options. I've explained my concerns in https://github.com/llvm/llvm-project/pull/123926#issuecomment-2614296866 https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -869,5 +875,12 @@ def convert_string(bytes_input): return str(bytes_input) +def print_filename(filename, null=False): createyourpersonalaccount wrote: I removed the default value of the argument in c47ce3ca8946f81f39757c82bfa5eb4cddc863a5. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/createyourpersonalaccount updated
https://github.com/llvm/llvm-project/pull/123926
>From 9dfbb9a3cc7f6bc557bc1ccf25cc727a02c4274c Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Wed, 22 Jan 2025 05:43:02 -0500
Subject: [PATCH 1/8] [clang-format] Add null-terminated path option (#123921)
This makes the `git clang-format` tool compose nicely with other shell
utilities in case of maliciously (or innocently) crafted filenames.
---
clang/tools/clang-format/git-clang-format | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index da271bbe6e3a07..04c49e8910d0ac 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -205,6 +205,12 @@ def main():
"commits"
),
)
+p.add_argument(
+"-0",
+"--null",
+action="store_true",
+help="print the affected paths with null-terminated characters",
+)
# We gather all the remaining positional arguments into 'args' since we
need
# to use some heuristics to determine whether or not was present.
# However, to print pretty messages, we make use of metavar and help.
@@ -261,11 +267,11 @@ def main():
"ignored by clang-format):"
)
for filename in ignored_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
if changed_lines:
print("Running clang-format on the following files:")
for filename in changed_lines:
-print("%s" % filename)
+print_filename(filename, opts.null)
if not changed_lines:
if opts.verbose >= 0:
@@ -304,7 +310,7 @@ def main():
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
print("changed files:")
for filename in changed_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
return 1
@@ -869,5 +875,12 @@ def convert_string(bytes_input):
return str(bytes_input)
+def print_filename(filename, null=False):
+if null:
+print(filename + "\0", end="")
+else:
+print("%s" % filename)
+
+
if __name__ == "__main__":
sys.exit(main())
>From 63424768ccd5cd2067448b7a86aeab16f01a0e78 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Fri, 24 Jan 2025 06:32:47 -0500
Subject: [PATCH 2/8] do not print anything but list of files when null is
enabled
---
clang/tools/clang-format/git-clang-format | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index 04c49e8910d0ac..9a26ef7a96a386 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -274,7 +274,7 @@ def main():
print_filename(filename, opts.null)
if not changed_lines:
-if opts.verbose >= 0:
+if opts.verbose >= 0 and not opts.null:
print("no modified files to format")
return 0
@@ -295,7 +295,7 @@ def main():
print("new tree: %s" % new_tree)
if old_tree == new_tree:
-if opts.verbose >= 0:
+if opts.verbose >= 0 and not opts.null:
print("clang-format did not modify any files")
return 0
@@ -308,7 +308,8 @@ def main():
old_tree, new_tree, force=opts.force, patch_mode=opts.patch
)
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
-print("changed files:")
+if not opts.null:
+print("changed files:")
for filename in changed_files:
print_filename(filename, opts.null)
>From 6b9460ab14abce7a3b2bda6851b920b64cfa9a67 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Sun, 26 Jan 2025 04:51:50 -0500
Subject: [PATCH 3/8] improve help string
Co-authored-by: Owen Pan
---
clang/tools/clang-format/git-clang-format | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index 9a26ef7a96a386..90917533cf3513 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -209,7 +209,7 @@ def main():
"-0",
"--null",
action="store_true",
-help="print the affected paths with null-terminated characters",
+help="end each printed filename with a null character",
)
# We gather all the remaining positional arguments into 'args' since we
need
# to use some heuristics to determine whether or not was present.
>From 343fa15be5246d429c22f611371ff8b678ff3355 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Sun, 26 Jan 2025 04:59:41 -0500
Subject: [PATCH 4/8] imp
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -205,6 +205,12 @@ def main(): "commits" ), ) +p.add_argument( +"-0", +"--null", +action="store_true", +help="print the affected paths with null-terminated characters", createyourpersonalaccount wrote: Done in e2ecb4bfc9dc8d43d7020d0d7476e86a6a29cadf, 9bd8510d2e4739dbb0347694c84360820a5343b1. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -205,6 +205,12 @@ def main(): "commits" ), ) +p.add_argument( +"-0", +"--null", createyourpersonalaccount wrote: Done in e2ecb4bfc9dc8d43d7020d0d7476e86a6a29cadf, 9bd8510d2e4739dbb0347694c84360820a5343b1. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -261,14 +267,14 @@ def main():
"ignored by clang-format):"
)
for filename in ignored_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
createyourpersonalaccount wrote:
Done in ab12a7ef5c3942c0992d8ee3c1df42a29a114627.
https://github.com/llvm/llvm-project/pull/123926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/createyourpersonalaccount updated
https://github.com/llvm/llvm-project/pull/123926
>From 9dfbb9a3cc7f6bc557bc1ccf25cc727a02c4274c Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Wed, 22 Jan 2025 05:43:02 -0500
Subject: [PATCH 1/7] [clang-format] Add null-terminated path option (#123921)
This makes the `git clang-format` tool compose nicely with other shell
utilities in case of maliciously (or innocently) crafted filenames.
---
clang/tools/clang-format/git-clang-format | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index da271bbe6e3a07..04c49e8910d0ac 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -205,6 +205,12 @@ def main():
"commits"
),
)
+p.add_argument(
+"-0",
+"--null",
+action="store_true",
+help="print the affected paths with null-terminated characters",
+)
# We gather all the remaining positional arguments into 'args' since we
need
# to use some heuristics to determine whether or not was present.
# However, to print pretty messages, we make use of metavar and help.
@@ -261,11 +267,11 @@ def main():
"ignored by clang-format):"
)
for filename in ignored_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
if changed_lines:
print("Running clang-format on the following files:")
for filename in changed_lines:
-print("%s" % filename)
+print_filename(filename, opts.null)
if not changed_lines:
if opts.verbose >= 0:
@@ -304,7 +310,7 @@ def main():
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
print("changed files:")
for filename in changed_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
return 1
@@ -869,5 +875,12 @@ def convert_string(bytes_input):
return str(bytes_input)
+def print_filename(filename, null=False):
+if null:
+print(filename + "\0", end="")
+else:
+print("%s" % filename)
+
+
if __name__ == "__main__":
sys.exit(main())
>From 63424768ccd5cd2067448b7a86aeab16f01a0e78 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Fri, 24 Jan 2025 06:32:47 -0500
Subject: [PATCH 2/7] do not print anything but list of files when null is
enabled
---
clang/tools/clang-format/git-clang-format | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index 04c49e8910d0ac..9a26ef7a96a386 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -274,7 +274,7 @@ def main():
print_filename(filename, opts.null)
if not changed_lines:
-if opts.verbose >= 0:
+if opts.verbose >= 0 and not opts.null:
print("no modified files to format")
return 0
@@ -295,7 +295,7 @@ def main():
print("new tree: %s" % new_tree)
if old_tree == new_tree:
-if opts.verbose >= 0:
+if opts.verbose >= 0 and not opts.null:
print("clang-format did not modify any files")
return 0
@@ -308,7 +308,8 @@ def main():
old_tree, new_tree, force=opts.force, patch_mode=opts.patch
)
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
-print("changed files:")
+if not opts.null:
+print("changed files:")
for filename in changed_files:
print_filename(filename, opts.null)
>From 6b9460ab14abce7a3b2bda6851b920b64cfa9a67 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Sun, 26 Jan 2025 04:51:50 -0500
Subject: [PATCH 3/7] improve help string
Co-authored-by: Owen Pan
---
clang/tools/clang-format/git-clang-format | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index 9a26ef7a96a386..90917533cf3513 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -209,7 +209,7 @@ def main():
"-0",
"--null",
action="store_true",
-help="print the affected paths with null-terminated characters",
+help="end each printed filename with a null character",
)
# We gather all the remaining positional arguments into 'args' since we
need
# to use some heuristics to determine whether or not was present.
>From 343fa15be5246d429c22f611371ff8b678ff3355 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Sun, 26 Jan 2025 04:59:41 -0500
Subject: [PATCH 4/7] imp
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/createyourpersonalaccount updated
https://github.com/llvm/llvm-project/pull/123926
>From 9dfbb9a3cc7f6bc557bc1ccf25cc727a02c4274c Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Wed, 22 Jan 2025 05:43:02 -0500
Subject: [PATCH 1/4] [clang-format] Add null-terminated path option (#123921)
This makes the `git clang-format` tool compose nicely with other shell
utilities in case of maliciously (or innocently) crafted filenames.
---
clang/tools/clang-format/git-clang-format | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index da271bbe6e3a07..04c49e8910d0ac 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -205,6 +205,12 @@ def main():
"commits"
),
)
+p.add_argument(
+"-0",
+"--null",
+action="store_true",
+help="print the affected paths with null-terminated characters",
+)
# We gather all the remaining positional arguments into 'args' since we
need
# to use some heuristics to determine whether or not was present.
# However, to print pretty messages, we make use of metavar and help.
@@ -261,11 +267,11 @@ def main():
"ignored by clang-format):"
)
for filename in ignored_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
if changed_lines:
print("Running clang-format on the following files:")
for filename in changed_lines:
-print("%s" % filename)
+print_filename(filename, opts.null)
if not changed_lines:
if opts.verbose >= 0:
@@ -304,7 +310,7 @@ def main():
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
print("changed files:")
for filename in changed_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
return 1
@@ -869,5 +875,12 @@ def convert_string(bytes_input):
return str(bytes_input)
+def print_filename(filename, null=False):
+if null:
+print(filename + "\0", end="")
+else:
+print("%s" % filename)
+
+
if __name__ == "__main__":
sys.exit(main())
>From 63424768ccd5cd2067448b7a86aeab16f01a0e78 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Fri, 24 Jan 2025 06:32:47 -0500
Subject: [PATCH 2/4] do not print anything but list of files when null is
enabled
---
clang/tools/clang-format/git-clang-format | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index 04c49e8910d0ac..9a26ef7a96a386 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -274,7 +274,7 @@ def main():
print_filename(filename, opts.null)
if not changed_lines:
-if opts.verbose >= 0:
+if opts.verbose >= 0 and not opts.null:
print("no modified files to format")
return 0
@@ -295,7 +295,7 @@ def main():
print("new tree: %s" % new_tree)
if old_tree == new_tree:
-if opts.verbose >= 0:
+if opts.verbose >= 0 and not opts.null:
print("clang-format did not modify any files")
return 0
@@ -308,7 +308,8 @@ def main():
old_tree, new_tree, force=opts.force, patch_mode=opts.patch
)
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
-print("changed files:")
+if not opts.null:
+print("changed files:")
for filename in changed_files:
print_filename(filename, opts.null)
>From 6b9460ab14abce7a3b2bda6851b920b64cfa9a67 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Sun, 26 Jan 2025 04:51:50 -0500
Subject: [PATCH 3/4] improve help string
Co-authored-by: Owen Pan
---
clang/tools/clang-format/git-clang-format | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index 9a26ef7a96a386..90917533cf3513 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -209,7 +209,7 @@ def main():
"-0",
"--null",
action="store_true",
-help="print the affected paths with null-terminated characters",
+help="end each printed filename with a null character",
)
# We gather all the remaining positional arguments into 'args' since we
need
# to use some heuristics to determine whether or not was present.
>From 343fa15be5246d429c22f611371ff8b678ff3355 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Sun, 26 Jan 2025 04:59:41 -0500
Subject: [PATCH 4/4] imp
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/createyourpersonalaccount updated
https://github.com/llvm/llvm-project/pull/123926
>From 9dfbb9a3cc7f6bc557bc1ccf25cc727a02c4274c Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Wed, 22 Jan 2025 05:43:02 -0500
Subject: [PATCH 1/3] [clang-format] Add null-terminated path option (#123921)
This makes the `git clang-format` tool compose nicely with other shell
utilities in case of maliciously (or innocently) crafted filenames.
---
clang/tools/clang-format/git-clang-format | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index da271bbe6e3a07..04c49e8910d0ac 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -205,6 +205,12 @@ def main():
"commits"
),
)
+p.add_argument(
+"-0",
+"--null",
+action="store_true",
+help="print the affected paths with null-terminated characters",
+)
# We gather all the remaining positional arguments into 'args' since we
need
# to use some heuristics to determine whether or not was present.
# However, to print pretty messages, we make use of metavar and help.
@@ -261,11 +267,11 @@ def main():
"ignored by clang-format):"
)
for filename in ignored_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
if changed_lines:
print("Running clang-format on the following files:")
for filename in changed_lines:
-print("%s" % filename)
+print_filename(filename, opts.null)
if not changed_lines:
if opts.verbose >= 0:
@@ -304,7 +310,7 @@ def main():
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
print("changed files:")
for filename in changed_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
return 1
@@ -869,5 +875,12 @@ def convert_string(bytes_input):
return str(bytes_input)
+def print_filename(filename, null=False):
+if null:
+print(filename + "\0", end="")
+else:
+print("%s" % filename)
+
+
if __name__ == "__main__":
sys.exit(main())
>From 63424768ccd5cd2067448b7a86aeab16f01a0e78 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Fri, 24 Jan 2025 06:32:47 -0500
Subject: [PATCH 2/3] do not print anything but list of files when null is
enabled
---
clang/tools/clang-format/git-clang-format | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index 04c49e8910d0ac..9a26ef7a96a386 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -274,7 +274,7 @@ def main():
print_filename(filename, opts.null)
if not changed_lines:
-if opts.verbose >= 0:
+if opts.verbose >= 0 and not opts.null:
print("no modified files to format")
return 0
@@ -295,7 +295,7 @@ def main():
print("new tree: %s" % new_tree)
if old_tree == new_tree:
-if opts.verbose >= 0:
+if opts.verbose >= 0 and not opts.null:
print("clang-format did not modify any files")
return 0
@@ -308,7 +308,8 @@ def main():
old_tree, new_tree, force=opts.force, patch_mode=opts.patch
)
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
-print("changed files:")
+if not opts.null:
+print("changed files:")
for filename in changed_files:
print_filename(filename, opts.null)
>From 6b9460ab14abce7a3b2bda6851b920b64cfa9a67 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Sun, 26 Jan 2025 04:51:50 -0500
Subject: [PATCH 3/3] improve help string
Co-authored-by: Owen Pan
---
clang/tools/clang-format/git-clang-format | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index 9a26ef7a96a386..90917533cf3513 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -209,7 +209,7 @@ def main():
"-0",
"--null",
action="store_true",
-help="print the affected paths with null-terminated characters",
+help="end each printed filename with a null character",
)
# We gather all the remaining positional arguments into 'args' since we
need
# to use some heuristics to determine whether or not was present.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
createyourpersonalaccount wrote: > From #123921, it seems that you only want the new option to work with > `--staged`, but should it also work with other options that may print a list > of filenames? I don't know; I only use `git clang-format` as in #123921 and I am not familiar with all its uses. I was hoping that you or someone other who is familiar could determine this. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -205,6 +205,12 @@ def main(): "commits" ), ) +p.add_argument( +"-0", +"--null", owenca wrote: Please move it (and `--diff_from_common_commit` above) up to keep the options sorted. I would use `--print0` (without `-0`) instead, just like in `find -print0` that can be piped to `xargs -0`. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -205,6 +205,12 @@ def main(): "commits" ), ) +p.add_argument( +"-0", +"--null", +action="store_true", +help="print the affected paths with null-terminated characters", owenca wrote: ```suggestion help="end each printed filename with a null character", ``` https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -869,5 +875,12 @@ def convert_string(bytes_input): return str(bytes_input) +def print_filename(filename, null=False): +if null: +print(filename + "\0", end="") owenca wrote: > Shouldn't you copy the the indentation of the printed files? It prints the filename, not the file. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -261,14 +267,14 @@ def main():
"ignored by clang-format):"
)
for filename in ignored_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
owenca wrote:
```suggestion
print_filenames(ignored_files, opts.print0)
```
and delete line 269 above.
https://github.com/llvm/llvm-project/pull/123926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -869,5 +876,12 @@ def convert_string(bytes_input):
return str(bytes_input)
+def print_filename(filename, null=False):
+if null:
+print(filename + "\0", end="")
+else:
+print("%s" % filename)
owenca wrote:
```suggestion
def print_filenames(filenames, print0=False):
for filename in filenames:
if print0:
print(filename, end="\0")
else:
print(" " * 4 + filename)
```
https://github.com/llvm/llvm-project/pull/123926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
owenca wrote: >From #123921, it seems that you only want the new option to work with >`--staged`, but should it also work with other options that may print a list >of filenames? https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/owenca edited https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -205,6 +205,12 @@ def main(): "commits" ), ) +p.add_argument( +"-0", +"--null", +action="store_true", +help="print the affected paths with null-terminated characters", HazardyKnusperkeks wrote: I think we should be consistent with the tools out there. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/createyourpersonalaccount updated
https://github.com/llvm/llvm-project/pull/123926
>From 9dfbb9a3cc7f6bc557bc1ccf25cc727a02c4274c Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Wed, 22 Jan 2025 05:43:02 -0500
Subject: [PATCH 1/2] [clang-format] Add null-terminated path option (#123921)
This makes the `git clang-format` tool compose nicely with other shell
utilities in case of maliciously (or innocently) crafted filenames.
---
clang/tools/clang-format/git-clang-format | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index da271bbe6e3a07..04c49e8910d0ac 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -205,6 +205,12 @@ def main():
"commits"
),
)
+p.add_argument(
+"-0",
+"--null",
+action="store_true",
+help="print the affected paths with null-terminated characters",
+)
# We gather all the remaining positional arguments into 'args' since we
need
# to use some heuristics to determine whether or not was present.
# However, to print pretty messages, we make use of metavar and help.
@@ -261,11 +267,11 @@ def main():
"ignored by clang-format):"
)
for filename in ignored_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
if changed_lines:
print("Running clang-format on the following files:")
for filename in changed_lines:
-print("%s" % filename)
+print_filename(filename, opts.null)
if not changed_lines:
if opts.verbose >= 0:
@@ -304,7 +310,7 @@ def main():
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
print("changed files:")
for filename in changed_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
return 1
@@ -869,5 +875,12 @@ def convert_string(bytes_input):
return str(bytes_input)
+def print_filename(filename, null=False):
+if null:
+print(filename + "\0", end="")
+else:
+print("%s" % filename)
+
+
if __name__ == "__main__":
sys.exit(main())
>From 63424768ccd5cd2067448b7a86aeab16f01a0e78 Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Fri, 24 Jan 2025 06:32:47 -0500
Subject: [PATCH 2/2] do not print anything but list of files when null is
enabled
---
clang/tools/clang-format/git-clang-format | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index 04c49e8910d0ac..9a26ef7a96a386 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -274,7 +274,7 @@ def main():
print_filename(filename, opts.null)
if not changed_lines:
-if opts.verbose >= 0:
+if opts.verbose >= 0 and not opts.null:
print("no modified files to format")
return 0
@@ -295,7 +295,7 @@ def main():
print("new tree: %s" % new_tree)
if old_tree == new_tree:
-if opts.verbose >= 0:
+if opts.verbose >= 0 and not opts.null:
print("clang-format did not modify any files")
return 0
@@ -308,7 +308,8 @@ def main():
old_tree, new_tree, force=opts.force, patch_mode=opts.patch
)
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
-print("changed files:")
+if not opts.null:
+print("changed files:")
for filename in changed_files:
print_filename(filename, opts.null)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -205,6 +205,12 @@ def main(): "commits" ), ) +p.add_argument( +"-0", +"--null", +action="store_true", +help="print the affected paths with null-terminated characters", createyourpersonalaccount wrote: > > I'm wondering if `-0` should turn off all informational messaging as well > > and _only_ print the paths. As it is, it will print things like _Running > > clang-format on the following files:_, even if a null-separated list of > > files follows. > > So right now the usage would be (example) > > ```shell > > git clang-format --staged -0 | tail -n +2 | xargs -0 git add > > ``` > > > > but perhaps it should not print informational messages and have the usage > > be simplified: > > ```shell > > git clang-format --staged -0 | xargs -0 git add > > ``` > > Aren't you really asking, just give me a list of all the files that have been > changed by clang format, in which case -0 isn't really that descriptive, what > not > > ``` > --modified or -m (similar to git ls-files) > ``` > > or `-- affected (might avoid becausee of affected/effected spelling issues > people have)` The list is already printed. The `-0` flag is printing a null separated list of files, see for example https://github.com/llvm/llvm-project/issues/123921. This allows to safely parse the list, avoiding issues with e.g. newlines in file names. > if you plan on dropping other text output (which I think perhaps you should > other wise it needs to be grep -v'd or tailed to make it work correctly, I > would perhaps rather we had a better named command line switch than -0 or > --null Okay, I will drop all other text except the list of files. Why do you say that `-0` and `--null` are bad? The standard is `-0` and `-z`, I think whichever works, but given that this will most likely be used with xargs, it makes sense to use `-0`. > I know this is likely how xargs or find works, but I think we can do a little > better than those tools in terms of clarity. What other tool is there? GNU Parallel also works with `-0` and generally any other tool that does what these tools do will have either `-0` or `-z`. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/mydeveloperday edited https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -205,6 +205,12 @@ def main(): "commits" ), ) +p.add_argument( +"-0", +"--null", +action="store_true", +help="print the affected paths with null-terminated characters", mydeveloperday wrote: if you plan on dropping other text output (which I think perhaps you should other wise it needs to be grep -v'd or tailed to make it work correctly, I would perhaps rather we had a better named command line switch than -0 or --null I know this is likely how xargs or find works, but I think we can do a little better than those tools in terms of clarity. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/mydeveloperday requested changes to this pull request. on reflection, I think better named arguments might be better https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
mydeveloperday wrote: > > This seems reasonable to me, but wait for the others, maybe @owenca > > I'm wondering if `-0` should turn off all informational messaging as well and > _only_ print the paths. As it is, it will print things like _Running > clang-format on the following files:_, even if a null-separated list of files > follows. > > So right now the usage would be (example) > > ```shell > git clang-format --staged -0 | tail -n +2 | xargs -0 git add > ``` > > but perhaps it should not print informational messages and have the usage be > simplified: > > ```shell > git clang-format --staged -0 | xargs -0 git add > ``` Aren't you really asking, just give me a list of all the files that have been changed by clang format, in which case -0 isn't really that descriptive, what not ``` --modified or -m (similar to git ls-files) ``` or `` -- affected (might avoid becausee of affected/effected spelling issues people have) `` https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
createyourpersonalaccount wrote: > > > This seems reasonable to me, but wait for the others, maybe @owenca > > > > > > I'm wondering if `-0` should turn off all informational messaging as well > > and _only_ print the paths. As it is, it will print things like _Running > > clang-format on the following files:_, even if a null-separated list of > > files follows. > > So right now the usage would be (example) > > ```shell > > git clang-format --staged -0 | tail -n +2 | xargs -0 git add > > ``` > > > > > > > > > > > > > > > > > > > > > > > > but perhaps it should not print informational messages and have the usage > > be simplified: > > ```shell > > git clang-format --staged -0 | xargs -0 git add > > ``` > > With that I can see why you don't indent. But you changed only the format of > the file name output. What else should change? https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -869,5 +875,12 @@ def convert_string(bytes_input): return str(bytes_input) +def print_filename(filename, null=False): HazardyKnusperkeks wrote: If this were C++ I'd say yes, you touched all calls (since you added it). Don't know if in Python it is done differently. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
HazardyKnusperkeks wrote: > > This seems reasonable to me, but wait for the others, maybe @owenca > > I'm wondering if `-0` should turn off all informational messaging as well and > _only_ print the paths. As it is, it will print things like _Running > clang-format on the following files:_, even if a null-separated list of files > follows. > > So right now the usage would be (example) > > ```shell > git clang-format --staged -0 | tail -n +2 | xargs -0 git add > ``` > > but perhaps it should not print informational messages and have the usage be > simplified: > > ```shell > git clang-format --staged -0 | xargs -0 git add > ``` With that I can see why you don't indent. But you changed only the format of the file name output. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
createyourpersonalaccount wrote: > This seems reasonable to me, but wait for the others, maybe @owenca I'm wondering if `-0` should turn off all informational messaging as well and *only* print the paths. As it is, it will print things like _Running clang-format on the following files:_, even if a null-separated list of files follows. So right now the usage would be (example) ```sh git clang-format --staged -z | tail -n +2 | xargs -0 git add ``` but perhaps it should not print informational messages and have the usage be simplified: ```sh git clang-format --staged -z | xargs -0 git add ``` https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/mydeveloperday approved this pull request. This seems reasonable to me, but wait for the others, maybe @owenca https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/createyourpersonalaccount edited https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -869,5 +875,12 @@ def convert_string(bytes_input): return str(bytes_input) +def print_filename(filename, null=False): createyourpersonalaccount wrote: I think that was a reflex, I set it to `False`, which is what `opts.null` is by default. Do you want me to change it to remove the default value? https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -869,5 +875,12 @@ def convert_string(bytes_input): return str(bytes_input) +def print_filename(filename, null=False): +if null: +print(filename + "\0", end="") createyourpersonalaccount wrote: I don't think so, because indentation is meaningless in binary output. You want to feed this to a program like `xargs -0`. This is useful when you want to pipe the binary data of a list of filepaths as-is to a tool. Adding tab characters in there would only confuse and make it necessary to crop the tab characters before piping it. https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -869,5 +875,12 @@ def convert_string(bytes_input): return str(bytes_input) +def print_filename(filename, null=False): HazardyKnusperkeks wrote: Why setting a value for null? https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
@@ -869,5 +875,12 @@ def convert_string(bytes_input): return str(bytes_input) +def print_filename(filename, null=False): +if null: +print(filename + "\0", end="") HazardyKnusperkeks wrote: Shouldn't you copy the the indentation of the printed files? https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
llvmbot wrote:
@llvm/pr-subscribers-clang-format
Author: Nikolaos Chatzikonstantinou (createyourpersonalaccount)
Changes
This makes the `git clang-format` tool compose nicely with other shell
utilities in case of maliciously (or innocently) crafted filenames.
---
Full diff: https://github.com/llvm/llvm-project/pull/123926.diff
1 Files Affected:
- (modified) clang/tools/clang-format/git-clang-format (+16-3)
``diff
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index da271bbe6e3a07..04c49e8910d0ac 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -205,6 +205,12 @@ def main():
"commits"
),
)
+p.add_argument(
+"-0",
+"--null",
+action="store_true",
+help="print the affected paths with null-terminated characters",
+)
# We gather all the remaining positional arguments into 'args' since we
need
# to use some heuristics to determine whether or not was present.
# However, to print pretty messages, we make use of metavar and help.
@@ -261,11 +267,11 @@ def main():
"ignored by clang-format):"
)
for filename in ignored_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
if changed_lines:
print("Running clang-format on the following files:")
for filename in changed_lines:
-print("%s" % filename)
+print_filename(filename, opts.null)
if not changed_lines:
if opts.verbose >= 0:
@@ -304,7 +310,7 @@ def main():
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
print("changed files:")
for filename in changed_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
return 1
@@ -869,5 +875,12 @@ def convert_string(bytes_input):
return str(bytes_input)
+def print_filename(filename, null=False):
+if null:
+print(filename + "\0", end="")
+else:
+print("%s" % filename)
+
+
if __name__ == "__main__":
sys.exit(main())
``
https://github.com/llvm/llvm-project/pull/123926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/123926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Add null-terminated path option (#123921) (PR #123926)
https://github.com/createyourpersonalaccount created
https://github.com/llvm/llvm-project/pull/123926
This makes the `git clang-format` tool compose nicely with other shell
utilities in case of maliciously (or innocently) crafted filenames.
>From 9dfbb9a3cc7f6bc557bc1ccf25cc727a02c4274c Mon Sep 17 00:00:00 2001
From: Nikolaos Chatzikonstantinou
Date: Wed, 22 Jan 2025 05:43:02 -0500
Subject: [PATCH] [clang-format] Add null-terminated path option (#123921)
This makes the `git clang-format` tool compose nicely with other shell
utilities in case of maliciously (or innocently) crafted filenames.
---
clang/tools/clang-format/git-clang-format | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/clang/tools/clang-format/git-clang-format
b/clang/tools/clang-format/git-clang-format
index da271bbe6e3a07..04c49e8910d0ac 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -205,6 +205,12 @@ def main():
"commits"
),
)
+p.add_argument(
+"-0",
+"--null",
+action="store_true",
+help="print the affected paths with null-terminated characters",
+)
# We gather all the remaining positional arguments into 'args' since we
need
# to use some heuristics to determine whether or not was present.
# However, to print pretty messages, we make use of metavar and help.
@@ -261,11 +267,11 @@ def main():
"ignored by clang-format):"
)
for filename in ignored_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
if changed_lines:
print("Running clang-format on the following files:")
for filename in changed_lines:
-print("%s" % filename)
+print_filename(filename, opts.null)
if not changed_lines:
if opts.verbose >= 0:
@@ -304,7 +310,7 @@ def main():
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
print("changed files:")
for filename in changed_files:
-print("%s" % filename)
+print_filename(filename, opts.null)
return 1
@@ -869,5 +875,12 @@ def convert_string(bytes_input):
return str(bytes_input)
+def print_filename(filename, null=False):
+if null:
+print(filename + "\0", end="")
+else:
+print("%s" % filename)
+
+
if __name__ == "__main__":
sys.exit(main())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
