All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Weimer <fweimer@redhat.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: Stephen Gallagher <sgallagh@redhat.com>,
	 netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-toolchains@vger.kernel.org
Subject: Re: [PATCH] iproute2: fix type incompatibility in ifstat.c
Date: Wed, 07 Feb 2024 07:44:59 +0100	[thread overview]
Message-ID: <877cjg6adw.fsf@oldenburg.str.redhat.com> (raw)
In-Reply-To: <20240206191209.3aaf9916@hermes.local> (Stephen Hemminger's message of "Tue, 6 Feb 2024 19:12:09 -0800")

* Stephen Hemminger:

> On Tue,  6 Feb 2024 09:22:06 -0500
> Stephen Gallagher <sgallagh@redhat.com> wrote:
>
>> Throughout ifstat.c, ifstat_ent.val is accessed as a long long unsigned
>> type, however it is defined as __u64. This works by coincidence on many
>> systems, however on ppc64le, __u64 is a long unsigned.
>> 
>> This patch makes the type definition consistent with all of the places
>> where it is accessed.
>> 
>> Signed-off-by: Stephen Gallagher <sgallagh@redhat.com>
>> ---

Patch was:

diff --git a/misc/ifstat.c b/misc/ifstat.c
index 721f4914..767cedd4 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -58,7 +58,7 @@ struct ifstat_ent {
 	struct ifstat_ent	*next;
 	char			*name;
 	int			ifindex;
-	__u64			val[MAXS];
+	unsigned long long	val[MAXS];
 	double			rate[MAXS];
 	__u32			ival[MAXS];
 };

> Why not fix the use of unsigned long long to be __u64 instead?
> That would make more sense.

You still won't be able to use %llu to print it.  I don't think the UAPI
headers provide anything like the <stdint.h> macros because the
assumption is that %llu is okay for printing __u64 on all architectures.

But we have this in POWER:

/*
 * This is here because we used to use l64 for 64bit powerpc
 * and we don't want to impact user mode with our change to ll64
 * in the kernel.
 *
 * However, some user programs are fine with this.  They can
 * flag __SANE_USERSPACE_TYPES__ to get int-ll64.h here.
 */
#if !defined(__SANE_USERSPACE_TYPES__) && defined(__powerpc64__) && !defined(__KERNEL__)
# include <asm-generic/int-l64.h>
#else
# include <asm-generic/int-ll64.h>
#endif

I didn't know some architectures are that … different.  Sadly this
wasn't fixed as part of the transition to powerpc64le.

I suppose iproute2 should build with -D__SANE_USERSPACE_TYPES__.

Thanks,
Florian


  reply	other threads:[~2024-02-07  6:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-06 14:22 [PATCH] iproute2: fix type incompatibility in ifstat.c Stephen Gallagher
2024-02-06 16:17 ` Andrea Claudi
2024-02-06 16:52 ` [PATCH iproute] Fix " Stephen Gallagher
2024-02-06 16:52   ` [PATCH] iproute2: fix " Stephen Gallagher
2024-02-15  3:10     ` patchwork-bot+netdevbpf
2024-02-07  3:12 ` Stephen Hemminger
2024-02-07  6:44   ` Florian Weimer [this message]
2024-02-08 18:17   ` Stephen Gallagher

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=877cjg6adw.fsf@oldenburg.str.redhat.com \
    --to=fweimer@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sgallagh@redhat.com \
    --cc=stephen@networkplumber.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.