| Message ID | 924d86933d2e2b6da6940f13e64ef0ab5008a797.1664095452.git.research_trasio@irq.a4lg.com |
|---|---|
| State | Superseded |
| Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8FE4F38582B1 for <patchwork@sourceware.org>; Sun, 25 Sep 2022 08:46:11 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8FE4F38582B1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1664095571; bh=DPu3oQYWJWYRnxsYlnQi53IXiEqt9z+EGH7mBMHD9Nw=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=jlbJZ9GdjKYSQ4i2n6o0urKUTmO3MGKG/dOOrX9VQuSYx2lbBE0mo5mDD7n5/463o nO6tArSfSi0fxZL7OX4zQgOnk7jkgwbEmDhniWZvkKGHBdBlyNRdTdfAiLfHlENWmP zuJWSAapfS3zeo+ozZXHBrraeLBrRIAoMKTqotQI= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-sender-0.a4lg.com (mail-sender.a4lg.com [153.120.152.154]) by sourceware.org (Postfix) with ESMTPS id A94393857B8D for <gdb-patches@sourceware.org>; Sun, 25 Sep 2022 08:45:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A94393857B8D Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id E704B300089; Sun, 25 Sep 2022 08:45:27 +0000 (UTC) To: Tsukasa OI <research_trasio@irq.a4lg.com>, Andrew Burgess <aburgess@redhat.com>, Mike Frysinger <vapier@gentoo.org>, Stephane Carrez <Stephane.Carrez@gmail.com>, "Frank Ch . Eigler" <fche@redhat.com> Subject: [PATCH 6/7] sim/ppc: Add ATTRIBUTE_PRINTF Date: Sun, 25 Sep 2022 08:44:16 +0000 Message-Id: <924d86933d2e2b6da6940f13e64ef0ab5008a797.1664095452.git.research_trasio@irq.a4lg.com> In-Reply-To: <cover.1664095452.git.research_trasio@irq.a4lg.com> References: <cover.1664095452.git.research_trasio@irq.a4lg.com> Mime-Version: 1.0 Content-Transfer-Encoding: 8bit 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 <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Tsukasa OI via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Tsukasa OI <research_trasio@irq.a4lg.com> Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
| Series | sim, sim/ARCH: Add ATTRIBUTE_PRINTF | |
Commit Message
Tsukasa OI
Sept. 25, 2022, 8:44 a.m. UTC
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(-)
Comments
Tsukasa OI <research_trasio@irq.a4lg.com> 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. 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 <stdlib.h> > #include <string.h> > > -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
On 2022/10/05 19:57, Andrew Burgess wrote: > Tsukasa OI <research_trasio@irq.a4lg.com> 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 <stdlib.h> >> #include <string.h> >> >> -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 >
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, ...) { 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 <stdlib.h> #include <string.h> -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;