All of lore.kernel.org
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: Borislav Petkov <bp@alien8.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: x86@kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
Date: Fri, 25 Jun 2021 09:19:38 -0700	[thread overview]
Message-ID: <F8B4FDC6-851F-4EC5-A308-BBAB52A75EF3@zytor.com> (raw)
In-Reply-To: <YNXkVBcmBvZL7khv@zn.tnic>

This is a user space build time tool.

You can use PRIu32/64 or cast to unsigned long long; it's not like the performance for this case is going to matter one iota.

On June 25, 2021 7:12:36 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
>On Fri, Jun 25, 2021 at 02:06:59PM +0200, Greg Kroah-Hartman wrote:
>> Casting seems to be the only way to make this "quiet" that I can
>tell.
>> 
>> Unless someone else has a good idea?
>
>Hmm, so in Documentation/core-api/printk-formats.rst we say that for
>printk() with different size types, we should "use a format specifier
>of
>its largest possible type and explicitly cast to it."
>
>And that kinda sounds ok to me because we don't potentially lose
>through
>the casting.
>
>IOW, I guess something like this below.
>
>---
>diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
>index 04c5a44b9682..42b0f425a2c7 100644
>--- a/arch/x86/tools/relocs.c
>+++ b/arch/x86/tools/relocs.c
>@@ -389,7 +389,7 @@ static void read_ehdr(FILE *fp)
> 		Elf_Shdr shdr;
> 
> 		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
>-			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
>+			die("Seek to %lu failed: %s\n", (unsigned long)ehdr.e_shoff,
>strerror(errno));
> 
> 		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
>			die("Cannot read initial ELF section header: %s\n",
>strerror(errno));
>@@ -412,17 +412,17 @@ static void read_shdrs(FILE *fp)
> 
> 	secs = calloc(shnum, sizeof(struct section));
> 	if (!secs) {
>-		die("Unable to allocate %d section headers\n",
>+		die("Unable to allocate %ld section headers\n",
> 		    shnum);
> 	}
> 	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0) {
>-		die("Seek to %d failed: %s\n",
>-			ehdr.e_shoff, strerror(errno));
>+		die("Seek to %lu failed: %s\n",
>+		    (unsigned long)ehdr.e_shoff, strerror(errno));
> 	}
> 	for (i = 0; i < shnum; i++) {
> 		struct section *sec = &secs[i];
> 		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
>-			die("Cannot read ELF section headers %d/%d: %s\n",
>+			die("Cannot read ELF section headers %d/%ld: %s\n",
> 			    i, shnum, strerror(errno));
> 		sec->shdr.sh_name      = elf_word_to_cpu(shdr.sh_name);
> 		sec->shdr.sh_type      = elf_word_to_cpu(shdr.sh_type);
>@@ -450,12 +450,12 @@ static void read_strtabs(FILE *fp)
> 		}
> 		sec->strtab = malloc(sec->shdr.sh_size);
> 		if (!sec->strtab) {
>-			die("malloc of %d bytes for strtab failed\n",
>-				sec->shdr.sh_size);
>+			die("malloc of %lu bytes for strtab failed\n",
>+			    (unsigned long)sec->shdr.sh_size);
> 		}
> 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
>-			die("Seek to %d failed: %s\n",
>-				sec->shdr.sh_offset, strerror(errno));
>+			die("Seek to %lu failed: %s\n",
>+			    (unsigned long)sec->shdr.sh_offset, strerror(errno));
> 		}
> 		if (fread(sec->strtab, 1, sec->shdr.sh_size, fp)
> 		    != sec->shdr.sh_size) {
>@@ -475,12 +475,12 @@ static void read_symtabs(FILE *fp)
> 		}
> 		sec->symtab = malloc(sec->shdr.sh_size);
> 		if (!sec->symtab) {
>-			die("malloc of %d bytes for symtab failed\n",
>-				sec->shdr.sh_size);
>+			die("malloc of %lu bytes for symtab failed\n",
>+			    (unsigned long)sec->shdr.sh_size);
> 		}
> 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
>-			die("Seek to %d failed: %s\n",
>-				sec->shdr.sh_offset, strerror(errno));
>+			die("Seek to %lu failed: %s\n",
>+			    (unsigned long)sec->shdr.sh_offset, strerror(errno));
> 		}
> 		if (fread(sec->symtab, 1, sec->shdr.sh_size, fp)
> 		    != sec->shdr.sh_size) {
>@@ -508,12 +508,12 @@ static void read_relocs(FILE *fp)
> 		}
> 		sec->reltab = malloc(sec->shdr.sh_size);
> 		if (!sec->reltab) {
>-			die("malloc of %d bytes for relocs failed\n",
>-				sec->shdr.sh_size);
>+			die("malloc of %lu bytes for relocs failed\n",
>+			    (unsigned long)sec->shdr.sh_size);
> 		}
> 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
>-			die("Seek to %d failed: %s\n",
>-				sec->shdr.sh_offset, strerror(errno));
>+			die("Seek to %lu failed: %s\n",
>+			    (unsigned long)sec->shdr.sh_offset, strerror(errno));
> 		}
> 		if (fread(sec->reltab, 1, sec->shdr.sh_size, fp)
> 		    != sec->shdr.sh_size) {
>diff --git a/arch/x86/tools/relocs.h b/arch/x86/tools/relocs.h
>index 43c83c0fd22c..4c49c82446eb 100644
>--- a/arch/x86/tools/relocs.h
>+++ b/arch/x86/tools/relocs.h
>@@ -17,6 +17,7 @@
> #include <regex.h>
> #include <tools/le_byteshift.h>
> 
>+__attribute__((__format__(printf, 1, 2)))
> void die(char *fmt, ...) __attribute__((noreturn));
> 
> #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

  reply	other threads:[~2021-06-25 16:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-24 11:58 [RESEND PATCH] x86/tools/relocs: add __printf attribute to die() Greg Kroah-Hartman
2021-06-24 12:32 ` Borislav Petkov
2021-06-24 14:44   ` Greg Kroah-Hartman
2021-06-25 16:17     ` H. Peter Anvin
2021-06-25 12:06   ` Greg Kroah-Hartman
2021-06-25 14:12     ` Borislav Petkov
2021-06-25 16:19       ` H. Peter Anvin [this message]
2021-06-25 16:53         ` Borislav Petkov
2021-06-25 20:38           ` H. Peter Anvin
2021-06-25 21:07             ` Borislav Petkov
2021-06-27 15:01               ` Greg Kroah-Hartman
2021-06-28 14:25                 ` [PATCH] x86/tools/relocs: Mark die() with the printf function attr format Borislav Petkov
2021-06-28 14:34                   ` Greg Kroah-Hartman
2021-08-23  5:12                   ` [tip: x86/build] " tip-bot2 for Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=F8B4FDC6-851F-4EC5-A308-BBAB52A75EF3@zytor.com \
    --to=hpa@zytor.com \
    --cc=bp@alien8.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.