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

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

[Carlo Marcelo Arenas Belón]

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

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

[Carlo Marcelo Arenas Belón]

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(+)

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

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

[Carlo Marcelo Arenas Belón]

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(-)

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

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

[Carlo Marcelo Arenas Belón]

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(-)

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

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

[Jeff Hostetler]

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!
Jeff

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

[Junio C Hamano]

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 ;-)

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

[Carlo Arenas]

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/

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

[Jeff Hostetler]

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 ;-)
> 

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

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

[Jeff Hostetler]

On 8/10/21 1:49 AM, Carlo Arenas wrote:
> 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

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

[Junio C Hamano]

Jeff Hostetler <git@jeffhostetler.com> writes:

> 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(+)

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

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

[Carlo Arenas]

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/

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

[Junio C Hamano]

Carlo Arenas <carenas@gmail.com> writes:

> 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.

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

[Carlo Arenas]

On Fri, Aug 27, 2021 at 10:11 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> 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/