From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender.a4lg.com [153.120.152.154]) by sourceware.org (Postfix) with ESMTPS id D451C384D14B for ; Thu, 6 Oct 2022 05:32:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D451C384D14B Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id 87272300089; Thu, 6 Oct 2022 05:32:41 +0000 (UTC) Message-ID: <04184772-e6c8-5bc7-dc48-ef95076417af@irq.a4lg.com> Date: Thu, 6 Oct 2022 14:32:39 +0900 Mime-Version: 1.0 Subject: Re: [PATCH 6/7] sim/ppc: Add ATTRIBUTE_PRINTF Content-Language: en-US To: Andrew Burgess Cc: gdb-patches@sourceware.org References: <924d86933d2e2b6da6940f13e64ef0ab5008a797.1664095452.git.research_trasio@irq.a4lg.com> <87mtaafsgk.fsf@redhat.com> From: Tsukasa OI In-Reply-To: <87mtaafsgk.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Oct 2022 05:32:46 -0000 On 2022/10/05 19:57, Andrew Burgess wrote: > Tsukasa OI writes: > >> Clang generates a warning if the format string of a printf-like function is >> not a literal ("-Wformat-nonliteral"). On the default configuration, it >> causes a build failure (unless "--disable-werror" is specified). >> >> To avoid warnings on the printf-like wrapper, it requires proper >> __attribute__((format)) and we have ATTRIBUTE_PRINTF macro for this reason. >> >> This commit adds ATTRIBUTE_PRINTF to the printf-like functions. >> >> sim/ChangeLog: >> >> * ppc/main.c (error): Add ATTRIBUTE_PRINTF. >> * ppc/misc.c (error, dumpf): Likewise. >> * ppc/sim_calls.c (error): Likewise. >> --- >> sim/ppc/main.c | 2 +- >> sim/ppc/misc.c | 4 ++-- >> sim/ppc/sim_calls.c | 2 +- >> 3 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/sim/ppc/main.c b/sim/ppc/main.c >> index 83b629ec14a..4a88166106f 100644 >> --- a/sim/ppc/main.c >> +++ b/sim/ppc/main.c >> @@ -68,7 +68,7 @@ sim_io_printf_filtered(const char *msg, ...) >> va_end(ap); >> } >> >> -void >> +void ATTRIBUTE_PRINTF(1, 2) >> error (const char *msg, ...) > > I notice in this patch, and the previous one, you've added > ATTRIBUTE_PRINTF to both the declaration, and the definition of some > functions. > > Is this required? I thought we only needed the attribute on the > declaration. > > In this case this difference is even more pronounced as you've added the > ATTRIBUTE_PRINTF, but the declaration also has ATTRIBUTE_NORETURN, which > you haven't added to the definition. > > My preference would be to only have the attributes on the declaration if > that is sufficient. Could you test that change and see if your build > issues are still resolved. Yes, declaration is sufficient. Because recent "build for Clang" patches are collection of many attempts so I think I mixed it somewhere. In the next version, I'll append this attribute to declarations, not definitions. Thanks, Tsukasa > > Thanks, > Andrew > > >> { >> va_list ap; >> diff --git a/sim/ppc/misc.c b/sim/ppc/misc.c >> index 8f2581e3ef3..71cda9fa298 100644 >> --- a/sim/ppc/misc.c >> +++ b/sim/ppc/misc.c >> @@ -28,7 +28,7 @@ >> #include >> #include >> >> -void >> +void ATTRIBUTE_PRINTF(1, 2) >> error (const char *msg, ...) >> { >> va_list ap; >> @@ -48,7 +48,7 @@ zalloc(long size) >> return memory; >> } >> >> -void >> +void ATTRIBUTE_PRINTF(2, 3) >> dumpf (int indent, const char *msg, ...) >> { >> va_list ap; >> diff --git a/sim/ppc/sim_calls.c b/sim/ppc/sim_calls.c >> index fbc327c94e0..b0ed3d4c3cc 100644 >> --- a/sim/ppc/sim_calls.c >> +++ b/sim/ppc/sim_calls.c >> @@ -388,7 +388,7 @@ sim_io_error (SIM_DESC sd, const char *fmt, ...) >> >> /****/ >> >> -void ATTRIBUTE_NORETURN >> +void ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF(1, 2) >> error (const char *msg, ...) >> { >> va_list ap; >> -- >> 2.34.1 >