threads / patch / 56240

patch, 2 partsfsmonitor: fixup to avoid warnings from pedantic

Subject: [PATCH 0/2] fsmonitor: fixup to avoid warnings from pedantic

## tl;dr

13 messages between Aug 9, 2021 and Aug 28, 2021. Diffs are folded; open one to read it.

replies: 12people: 3as markdown or json

Carlo Marcelo Arenas Belón· Aug 9, 2021, 06:30 UTC · lore

seen with both clang 12 and gcc 11 in seen (including jh/builtin-fsmonitor) as of a5a42b9f76 (from Jul 1)

it should be squashed into the corresponding patch from the series as shown in the subject and reroll IMHO; notice Junio also has some bandaid sparse fixes for t/helper/test-touch.c that should be included.

a similar additional fix for C89 compatibility to be send as a reply later
Carlo Marcelo Arenas Belón (2):
  fixup! fsmonitor-ipc: create client routines for git-fsmonitor--daemon
  fixup! fsmonitor--daemon: implement handle_client callback
 builtin/fsmonitor--daemon.c | 2 +-
 fsmonitor-ipc.h             | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)
-- 
2.33.0.rc1.373.gc715f1a457
Carlo Marcelo Arenas Belón· Aug 9, 2021, 06:30 UTC · re: Carlo Marcelo Arenas Belón · lore

[PATCH 1/2] fixup! fsmonitor-ipc: create client routines for git-fsmonitor--daemon

fsmonitor: ensure enum ipc_active_state is defined before using
In file included from fsmonitor.c:6:
fsmonitor-ipc.h:23:6: error: ISO C forbids forward references to 'enum' types [-Werror=pedantic]
   23 | enum ipc_active_state fsmonitor_ipc__get_state(void);
      |      ^~~~~~~~~~~~~~~~
In file included from t/helper/test-fsmonitor-client.c:9:
./fsmonitor-ipc.h:23:6: warning: ISO C forbids forward references to 'enum' types [-Wpedantic]
enum ipc_active_state fsmonitor_ipc__get_state(void);
     ^
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 fsmonitor-ipc.h | 2 ++
 1 file changed, 2 insertions(+)
Show changes to fsmonitor-ipc.h +2 −0
diff --git a/fsmonitor-ipc.h b/fsmonitor-ipc.h
index 837c5e5b64..7e02c7285a 100644
--- a/fsmonitor-ipc.h
+++ b/fsmonitor-ipc.h
@@ -1,6 +1,8 @@
 #ifndef FSMONITOR_IPC_H
 #define FSMONITOR_IPC_H
 
+#include "simple-ipc.h"
+
 /*
  * Returns true if built-in file system monitor daemon is defined
  * for this platform.

-- 
2.33.0.rc1.373.gc715f1a457
Carlo Marcelo Arenas Belón· Aug 9, 2021, 06:30 UTC · re: Carlo Marcelo Arenas Belón · lore

[PATCH 2/2] fixup! fsmonitor--daemon: implement handle_client callback

fsmonitor: avoid extra ; for KHASH_INIT
builtin/fsmonitor--daemon.c:622:75: error: ISO C does not allow extra ';' outside of a function [-Werror=pedantic]
  622 | KHASH_INIT(str, const char *, int, 0, kh_str_hash_func, kh_str_hash_equal);
      |                                                                           ^
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 builtin/fsmonitor--daemon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to builtin/fsmonitor--daemon.c +1 −1
diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
index 25f18f2726..d018af346f 100644
--- a/builtin/fsmonitor--daemon.c
+++ b/builtin/fsmonitor--daemon.c
@@ -619,7 +619,7 @@ static int fsmonitor_parse_client_token(const char *buf_token,
 	return 0;
 }
 
-KHASH_INIT(str, const char *, int, 0, kh_str_hash_func, kh_str_hash_equal);
+KHASH_INIT(str, const char *, int, 0, kh_str_hash_func, kh_str_hash_equal)
 
 static int do_handle_client(struct fsmonitor_daemon_state *state,
 			    const char *command,

-- 
2.33.0.rc1.373.gc715f1a457
Carlo Marcelo Arenas Belón· Aug 9, 2021, 07:25 UTC · re: Carlo Marcelo Arenas Belón · lore

[PATCH] test-path-utils: use a C89 compatible format for double printf

a62f9d1ace (test-path-utils: offer to run a protectNTFS/protectHFS benchmark, 2019-09-04) add a tool for testing that uses a double variable to keep track of time but prints them with "%lf".

while correct for newer C standards, it is not defined in C89 and it is not needed, since both float and double will be promoted to double by printf and should therefore use "%f"; change it accordingly.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 t/helper/test-path-utils.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to t/helper/test-path-utils.c +3 −3
diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index 229ed416b0..3d33d2a3e7 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -275,18 +275,18 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
 				for (j = 0; j < nr; j++)
 					verify_path(names[j], file_mode);
 				end = getnanotime();
-				printf("protect_ntfs = %d, protect_hfs = %d: %lfms\n", protect_ntfs, protect_hfs, (end-begin) / (double)1e6);
+				printf("protect_ntfs = %d, protect_hfs = %d: %fms\n", protect_ntfs, protect_hfs, (end-begin) / (double)1e6);
 				cumul += end - begin;
 				cumul2 += (end - begin) * (end - begin);
 			}
 			m[protect_ntfs][protect_hfs] = cumul / (double)repetitions;
 			v[protect_ntfs][protect_hfs] = my_sqrt(cumul2 / (double)repetitions - m[protect_ntfs][protect_hfs] * m[protect_ntfs][protect_hfs]);
-			printf("mean: %lfms, stddev: %lfms\n", m[protect_ntfs][protect_hfs] / (double)1e6, v[protect_ntfs][protect_hfs] / (double)1e6);
+			printf("mean: %fms, stddev: %fms\n", m[protect_ntfs][protect_hfs] / (double)1e6, v[protect_ntfs][protect_hfs] / (double)1e6);
 		}
 
 	for (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)
 		for (protect_hfs = 0; protect_hfs < 2; protect_hfs++)
-			printf("ntfs=%d/hfs=%d: %lf%% slower\n", protect_ntfs, protect_hfs, (m[protect_ntfs][protect_hfs] - m[0][0]) * 100 / m[0][0]);
+			printf("ntfs=%d/hfs=%d: %f%% slower\n", protect_ntfs, protect_hfs, (m[protect_ntfs][protect_hfs] - m[0][0]) * 100 / m[0][0]);
 
 	return 0;
 }

-- 
2.33.0.rc1.373.gc715f1a457
Jeff Hostetler· Aug 9, 2021, 14:34 UTC · re: Carlo Marcelo Arenas Belón · lore

Re: [PATCH 0/2] fsmonitor: fixup to avoid warnings from pedantic

On 8/9/21 2:30 AM, Carlo Marcelo Arenas Belón wrote:
Show 17 quoted lines
> seen with both clang 12 and gcc 11 in seen (including jh/builtin-fsmonitor)
> as of a5a42b9f76 (from Jul 1)
> 
> it should be squashed into the corresponding patch from the series as shown
> in the subject and reroll IMHO; notice Junio also has some bandaid sparse
> fixes for t/helper/test-touch.c that should be included.
> 
> a similar additional fix for C89 compatibility to be send as a reply later
> 
> Carlo Marcelo Arenas Belón (2):
>    fixup! fsmonitor-ipc: create client routines for git-fsmonitor--daemon
>    fixup! fsmonitor--daemon: implement handle_client callback
> 
>   builtin/fsmonitor--daemon.c | 2 +-
>   fsmonitor-ipc.h             | 2 ++
>   2 files changed, 3 insertions(+), 1 deletion(-)
>

Thanks! Jeff

Junio C Hamano· Aug 9, 2021, 16:49 UTC · re: Jeff Hostetler · lore

Re: [PATCH 0/2] fsmonitor: fixup to avoid warnings from pedantic

Jeff Hostetler <git@jeffhostetler.com> writes:
Show 18 quoted lines
> On 8/9/21 2:30 AM, Carlo Marcelo Arenas Belón wrote:
>> seen with both clang 12 and gcc 11 in seen (including jh/builtin-fsmonitor)
>> as of a5a42b9f76 (from Jul 1)
>> it should be squashed into the corresponding patch from the series
>> as shown
>> in the subject and reroll IMHO; notice Junio also has some bandaid sparse
>> fixes for t/helper/test-touch.c that should be included.
>> a similar additional fix for C89 compatibility to be send as a reply
>> later
>> Carlo Marcelo Arenas Belón (2):
>>    fixup! fsmonitor-ipc: create client routines for git-fsmonitor--daemon
>>    fixup! fsmonitor--daemon: implement handle_client callback
>>   builtin/fsmonitor--daemon.c | 2 +-
>>   fsmonitor-ipc.h             | 2 ++
>>   2 files changed, 3 insertions(+), 1 deletion(-)
>> 
>
> Thanks!
Thanks, both.

Jeff, I understand that this is planned to be rerolled post release, so I'll not touch these fixups and let yuou worry about them ;-)

Carlo Arenas· Aug 10, 2021, 05:49 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/2] fsmonitor: fixup to avoid warnings from pedantic

On Mon, Aug 9, 2021 at 9:49 AM Junio C Hamano <gitster@pobox.com> wrote:
> Jeff, I understand that this is planned to be rerolled post release,
> so I'll not touch these fixups and let you worry about them ;-)
Jeff,

As I mentioned in the cover letter[1] there is an extra patch[2] not in the series and that also applies to maint, but is not critical and might be better if included with your reroll IMHO, thanks.

Carlo

[1] https://lore.kernel.org/git/20210809063004.73736-1-carenas@gmail.com/ [2] https://lore.kernel.org/git/20210809072551.94391-1-carenas@gmail.com/

Jeff Hostetler· Aug 10, 2021, 13:52 UTC · re: Carlo Arenas · lore

Re: [PATCH 0/2] fsmonitor: fixup to avoid warnings from pedantic

On 8/10/21 1:49 AM, Carlo Arenas wrote:
Show 15 quoted lines
> On Mon, Aug 9, 2021 at 9:49 AM Junio C Hamano <gitster@pobox.com> wrote:
>> Jeff, I understand that this is planned to be rerolled post release,
>> so I'll not touch these fixups and let you worry about them ;-)
> 
> Jeff,
> 
> As I mentioned in the cover letter[1] there is an extra patch[2] not
> in the series and that also applies to maint, but is not critical and
> might be better if included with your reroll IMHO, thanks.
> 
> Carlo
> 
> [1] https://lore.kernel.org/git/20210809063004.73736-1-carenas@gmail.com/
> [2] https://lore.kernel.org/git/20210809072551.94391-1-carenas@gmail.com/
> 
You're talking about the commit with "%lf" vs "%f".

I saw that, but I was unsure what it had to do with my FSMonitor series. It seemed like it could just be a standalone patch by itself.

(I'm not opposed to including it in mine, but it'd be easier and quicker to just treat it in isolation, IMHO.)

Thoughts??
Jeff
Jeff Hostetler· Aug 10, 2021, 13:41 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/2] fsmonitor: fixup to avoid warnings from pedantic

On 8/9/21 12:49 PM, Junio C Hamano wrote:
Show 26 quoted lines
> Jeff Hostetler <git@jeffhostetler.com> writes:
> 
>> On 8/9/21 2:30 AM, Carlo Marcelo Arenas Belón wrote:
>>> seen with both clang 12 and gcc 11 in seen (including jh/builtin-fsmonitor)
>>> as of a5a42b9f76 (from Jul 1)
>>> it should be squashed into the corresponding patch from the series
>>> as shown
>>> in the subject and reroll IMHO; notice Junio also has some bandaid sparse
>>> fixes for t/helper/test-touch.c that should be included.
>>> a similar additional fix for C89 compatibility to be send as a reply
>>> later
>>> Carlo Marcelo Arenas Belón (2):
>>>     fixup! fsmonitor-ipc: create client routines for git-fsmonitor--daemon
>>>     fixup! fsmonitor--daemon: implement handle_client callback
>>>    builtin/fsmonitor--daemon.c | 2 +-
>>>    fsmonitor-ipc.h             | 2 ++
>>>    2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>
>> Thanks!
> 
> Thanks, both.
> 
> Jeff, I understand that this is planned to be rerolled post release,
> so I'll not touch these fixups and let yuou worry about them ;-)
>

Right. I'll include them with my next version. Jeff

Junio C Hamano· Aug 27, 2021, 05:37 UTC · re: Jeff Hostetler · lore

Re: [PATCH 0/2] fsmonitor: fixup to avoid warnings from pedantic

Jeff Hostetler <git@jeffhostetler.com> writes:
Show 24 quoted lines
> On 8/9/21 12:49 PM, Junio C Hamano wrote:
>> Jeff Hostetler <git@jeffhostetler.com> writes:
>> 
>>> On 8/9/21 2:30 AM, Carlo Marcelo Arenas Belón wrote:
>>>> seen with both clang 12 and gcc 11 in seen (including jh/builtin-fsmonitor)
>>>> as of a5a42b9f76 (from Jul 1)
>>>> it should be squashed into the corresponding patch from the series
>>>> as shown
>>>> in the subject and reroll IMHO; notice Junio also has some bandaid sparse
>>>> fixes for t/helper/test-touch.c that should be included.
>>>> a similar additional fix for C89 compatibility to be send as a reply
>>>> later
>>>> Carlo Marcelo Arenas Belón (2):
>>>>     fixup! fsmonitor-ipc: create client routines for git-fsmonitor--daemon
>>>>     fixup! fsmonitor--daemon: implement handle_client callback
>>>>    builtin/fsmonitor--daemon.c | 2 +-
>>>>    fsmonitor-ipc.h             | 2 ++
>>>>    2 files changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>
>>> Thanks!
>> Thanks, both.
>> Jeff, I understand that this is planned to be rerolled post release,
>> so I'll not touch these fixups and let yuou worry about them ;-)
Here is what I had to do to work around
 https://github.com/git/git/runs/3438543601?check_suite_focus=true#step:5:136

I also have another SQUASH??? fix queued on the topic, but I think you've already seen it.

---
 fsmonitor-ipc.h | 2 ++
 1 file changed, 2 insertions(+)
Show changes to fsmonitor-ipc.h +2 −0
diff --git a/fsmonitor-ipc.h b/fsmonitor-ipc.h
index 837c5e5b64..7e02c7285a 100644
--- a/fsmonitor-ipc.h
+++ b/fsmonitor-ipc.h
@@ -1,6 +1,8 @@
 #ifndef FSMONITOR_IPC_H
 #define FSMONITOR_IPC_H
 
+#include "simple-ipc.h"
+
 /*
  * Returns true if built-in file system monitor daemon is defined
  * for this platform.

-- 
2.33.0-205-g1b5b9d966b
Carlo Arenas· Aug 27, 2021, 06:04 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/2] fsmonitor: fixup to avoid warnings from pedantic

On Thu, Aug 26, 2021 at 10:37 PM Junio C Hamano <gitster@pobox.com> wrote:
> Here is what I had to do to work around
>
>  https://github.com/git/git/runs/3438543601?check_suite_focus=true#step:5:136
correct; that was 1/2 of the fixups I proposed and were waiting for the reroll.

interesting that the job was able to build without 2/2[1] but it might be because clang is even more pedantic.

Carlo
[1] https://lore.kernel.org/git/20210809063004.73736-3-carenas@gmail.com/
Junio C Hamano· Aug 27, 2021, 17:11 UTC · re: Carlo Arenas · lore

Re: [PATCH 0/2] fsmonitor: fixup to avoid warnings from pedantic

Carlo Arenas <carenas@gmail.com> writes:
Show 13 quoted lines
> On Thu, Aug 26, 2021 at 10:37 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Here is what I had to do to work around
>>
>>  https://github.com/git/git/runs/3438543601?check_suite_focus=true#step:5:136
>
> correct; that was 1/2 of the fixups I proposed and were waiting for the reroll.
>
> interesting that the job was able to build without 2/2[1] but it might
> be because clang is even more pedantic.
>
> Carlo
>
> [1] https://lore.kernel.org/git/20210809063004.73736-3-carenas@gmail.com/

One thing I noticed is that simple-ipc.h can disable itself altogether with "#ifdef SUPPORTS_SIMPLE_IPC", so the unconditional use of "enum ipc_active_state" by fsmonitor-ipc.h would still be a problem. fsmonitor-ipc.h is included even in help.c so it seems that with this topic, you practically will not get a working Git at all with -pedantic when SUPPORTS_SIMPLE_IPC is not true.

Carlo Arenas· Aug 28, 2021, 09:45 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/2] fsmonitor: fixup to avoid warnings from pedantic

On Fri, Aug 27, 2021 at 10:11 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 5 quoted lines
>
> One thing I noticed is that simple-ipc.h can disable itself
> altogether with "#ifdef SUPPORTS_SIMPLE_IPC", so the unconditional
> use of "enum ipc_active_state" by fsmonitor-ipc.h would still be a
> problem.

That is why we also need an additional patch[1] to be able to build (at least in macOS).

in windows additional changes are required, but I suspect that configuration is probably not expected.

> fsmonitor-ipc.h is included even in help.c so it seems
> that with this topic, you practically will not get a working Git at
> all with -pedantic when SUPPORTS_SIMPLE_IPC is not true.

not sure if I understand the logic here, but there is a "mock" daemon that is used in the platforms that don't have support that will be available then (even if it just refuses to start), and that is still IMHO a working git configuration.

Carlo
[1] https://lore.kernel.org/git/20210817082310.2157-1-carenas@gmail.com/

← back to recent threads