On Fri, Apr 15 2022, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> On Fri, Apr 15 2022, Carlo Arenas wrote: >> >>>> > +# https://bugzilla.redhat.com/show_bug.cgi?id=2075786 >>>> > +ifneq ($(filter gcc12,$(COMPILER_FEATURES)),) >>>> > +DEVELOPER_CFLAGS += -Wno-error=stringop-overread >>>> > +endif >>>> >>>> What I meant with "just set -Wno-error=stringop-overread on gcc12 for >>>> dir.(o|s|sp)?" was that you can set this per-file: >>> >>> of course, but that change goes in the Makefile and therefore affects >>> ... >> I mean it can go in config.mak.dev, it doesn't need to be in the >> Makefile itself. >> ... >>>> dir.sp dir.s dir.o: EXTRA_CPPFLAGS += -Wno-error=stringop-overread >>> >>> I know at least one developer that will then rightfully complain that >>> the git build doesn't work in AIX with xl after this. >> >> Yes, it would break if it were in the Makfile, but not if it's in >> config.mak.dev. > > I do not think you can blame Carlo for poor reading/comprehension in > this case---I too (mis)read what you wrote, and didn't realize that > you were suggesting to add the "for these target, EXTRA_CPPFLAGS > additionally gets this value" inside the ifneq/endif Carlo added to > hold the DEVELOPER_CFLAGS thing. Indeed, I don't think I would have understood myself, I didn't mean to imply any fault (except my own for not elaborating). Just claifying that we can use that trick. I.e. my own config.mak has had this (or a form thereof) for a while: http.sp http.s http.o: EXTRA_CPPFLAGS += -Wno-error=dangling-pointer= dir.sp dir.s dir.o: EXTRA_CPPFLAGS += -Wno-error=stringop-overread > For now, let's stick to the simpler form, though. Sure, works for me. Note though that one important difference between this solution and the patch I had for http.c is that the patch will fix things for all builds, whereas a config.mak.dev change (whether it's Carlos's global addition, or my per-file) can only do so for cases where DEVELOPER=1. Although in practice that's probably fine, anyone turning on -Werror is likely to do so through DEVELOPER=1, and fore those that don't it's "just a warning". So yeah, it's probably better to do that for now. But FWIW I did write up and test the below monstrosity as a replacement just now, it's guaranteed to work with/without DEVELOPER, and squashes only that specific warning, and only on GCC: diff --git a/dir.c b/dir.c index f2b0f242101..e7a5acb126f 100644 --- a/dir.c +++ b/dir.c @@ -3089,6 +3089,13 @@ char *git_url_basename(const char *repo, int is_bundle, int is_bare) * result in a dir '2222' being guessed due to backwards * compatibility. */ +#ifdef __clang__ +#elif defined(__GNUC__) +#if __GNUC__ >= 12 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wstringop-overread" +#endif +#endif if (memchr(start, '/', end - start) == NULL && memchr(start, ':', end - start) != NULL) { ptr = end; @@ -3097,6 +3104,12 @@ char *git_url_basename(const char *repo, int is_bundle, int is_bare) if (start < ptr && ptr[-1] == ':') end = ptr - 1; } +#ifdef __clang__ +#elif defined(__GNUC__) +#if __GNUC__ >= 12 +#pragma GCC diagnostic pop +#endif +#endif /* * Find last component. To remain backwards compatible we diff --git a/http.c b/http.c index 229da4d1488..e63d4ab9527 100644 --- a/http.c +++ b/http.c @@ -1329,7 +1329,21 @@ void run_active_slot(struct active_request_slot *slot) struct timeval select_timeout; int finished = 0; +#ifdef __clang__ +#elif defined(__GNUC__) +#if __GNUC__ >= 12 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdangling-pointer=" +#endif +#endif slot->finished = &finished; +#ifdef __clang__ +#elif defined(__GNUC__) +#if __GNUC__ >= 12 +#pragma GCC diagnostic pop +#endif +#endif + while (!finished) { step_active_slots(); But yeah, between us not having -Werror by default and DEVELOPER handling it it's probably not worth it just to be able to selectively suppress the warnings involved.
