New bcache compiler warning (was: Re: bcache: Minor fixes from kbuild robot)

Post by Geert Uytterhoeven
On Fri, Feb 14, 2014 at 8:04 PM, Linux Kernel Mailing List

bcache: Minor fixes from kbuild robot

=20

diff --git a/drivers/md/bcache/bset.c b/drivers/md/bcache/bset.c
index 4f6b594..3f74b4b 100644
--- a/drivers/md/bcache/bset.c
+++ b/drivers/md/bcache/bset.c
@@ -23,7 +23,7 @@ void bch_dump_bset(struct btree_keys *b, struct bs=

et *i, unsigned set)

=20
drivers/md/bcache/bset.c: In function =E2=80=98bch_dump_bset=E2=80=99=
drivers/md/bcache/bset.c:27: warning: format =E2=80=98%li=E2=80=99 ex=

pects type =E2=80=98long

Post by Geert Uytterhoeven
int=E2=80=99, but argument 3 has type =E2=80=98int=E2=80=99
=20
What are you trying to print here? It looks a bit strange to me.
Technically, the difference between two pointers is of type ptrdiff_.
The kernel had
=20
typedef __kernel_ptrdiff_t ptrdiff_t;
=20
and
=20
#if __BITS_PER_LONG !=3D 64
typedef unsigned int __kernel_size_t;
typedef int __kernel_ssize_t;
typedef int __kernel_ptrdiff_t;
#else
typedef __kernel_ulong_t __kernel_size_t;
typedef __kernel_long_t __kernel_ssize_t;
typedef __kernel_long_t __kernel_ptrdiff_t;
#endif
=20
So I'd expect "%zi" to be the right way, and a quick test compile on
32-bit (m68k)
and 64-bit (amd64) comfirms that. What was wrong with it?

The kernel supports 't' (%t) for ptrdiff_t (same as glibc),
so %ti should work (or %tu).

--=20
~Randy

Geert Uytterhoeven

2014-02-17 21:11:51 UTC

Post by Geert Uytterhoeven
On Fri, Feb 14, 2014 at 8:04 PM, Linux Kernel Mailing List

set *i, unsigned set)

drivers/md/bcache/bset.c: In function =E2=80=98bch_dump_bset=E2=80=99=
drivers/md/bcache/bset.c:27: warning: format =E2=80=98%li=E2=80=99 e=

xpects type =E2=80=98long

Post by Geert Uytterhoeven
int=E2=80=99, but argument 3 has type =E2=80=98int=E2=80=99
What are you trying to print here? It looks a bit strange to me.
Technically, the difference between two pointers is of type ptrdiff_=

=2E

Post by Geert Uytterhoeven
The kernel had
typedef __kernel_ptrdiff_t ptrdiff_t;
and
#if __BITS_PER_LONG !=3D 64
typedef unsigned int __kernel_size_t;
typedef int __kernel_ssize_t;
typedef int __kernel_ptrdiff_t;
#else
typedef __kernel_ulong_t __kernel_size_t;
typedef __kernel_long_t __kernel_ssize_t;
typedef __kernel_long_t __kernel_ptrdiff_t;
#endif
So I'd expect "%zi" to be the right way, and a quick test compile on
32-bit (m68k)
and 64-bit (amd64) comfirms that. What was wrong with it?

The kernel supports 't' (%t) for ptrdiff_t (same as glibc),
so %ti should work (or %tu).

Yes, that compiles without warnings, too.

And after more decyphering, "(uint64_t *) k - i->d" seems to be positiv=
e,
so "%tu" should be OK.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ***@linux-=
m68k.org

In personal conversations with technical people, I call myself a hacker=
=2E But
when I'm talking to journalists I just say "programmer" or something li=
ke that.
-- Linus Torvalds

Kent Overstreet

2014-02-17 21:45:53 UTC

Post by Geert Uytterhoeven
On Fri, Feb 14, 2014 at 8:04 PM, Linux Kernel Mailing List

bset *i, unsigned set)

drivers/md/bcache/bset.c: In function =E2=80=98bch_dump_bset=E2=80=
drivers/md/bcache/bset.c:27: warning: format =E2=80=98%li=E2=80=99=

expects type =E2=80=98long

f_.

Post by Geert Uytterhoeven
32-bit (m68k)
and 64-bit (amd64) comfirms that. What was wrong with it?

The kernel supports 't' (%t) for ptrdiff_t (same as glibc),
so %ti should work (or %tu).

=20
Yes, that compiles without warnings, too.
=20
And after more decyphering, "(uint64_t *) k - i->d" seems to be posit=

ive,

Post by Geert Uytterhoeven
so "%tu" should be OK.

*swears* Actually, I'm just going to cast this to unsigned (that's defi=
nitely
safe here):

commit 70bc49d421c793f73a772ae1f50622a39c6136d9
Author: Kent Overstreet <***@daterainc.com>
Date: Mon Feb 17 13:44:06 2014 -0800

bcache: Fix another compiler warning on m68k
=20
Use a bigger hammer this time
=20
Signed-off-by: Kent Overstreet <***@daterainc.com>

diff --git a/drivers/md/bcache/bset.c b/drivers/md/bcache/bset.c
index 3f74b4b074..5454164153 100644
--- a/drivers/md/bcache/bset.c
+++ b/drivers/md/bcache/bset.c
@@ -23,8 +23,8 @@ void bch_dump_bset(struct btree_keys *b, struct bset =
*i, unsigned set)
for (k =3D i->start; k < bset_bkey_last(i); k =3D next) {
next =3D bkey_next(k);
=20
- printk(KERN_ERR "block %u key %li/%u: ", set,
- (uint64_t *) k - i->d, i->keys);
+ printk(KERN_ERR "block %u key %u/%u: ", set,
+ (unsigned) ((u64 *) k - i->d), i->keys);
=20
if (b->ops->key_dump)
b->ops->key_dump(b, k);

Randy Dunlap

2014-02-17 22:44:47 UTC

Post by Geert Uytterhoeven
On Fri, Feb 14, 2014 at 8:04 PM, Linux Kernel Mailing List

bset *i, unsigned set)

drivers/md/bcache/bset.c:27: warning: format =91%li=92 expects typ=

e =91long

Post by Geert Uytterhoeven
int=92, but argument 3 has type =91int=92
What are you trying to print here? It looks a bit strange to me.
Technically, the difference between two pointers is of type ptrdif=

f_.

Post by Geert Uytterhoeven
32-bit (m68k)
and 64-bit (amd64) comfirms that. What was wrong with it?

The kernel supports 't' (%t) for ptrdiff_t (same as glibc),
so %ti should work (or %tu).

Yes, that compiles without warnings, too.
And after more decyphering, "(uint64_t *) k - i->d" seems to be posi=

tive,

Post by Geert Uytterhoeven
so "%tu" should be OK.

=20
*swears* Actually, I'm just going to cast this to unsigned (that's de=

finitely

=20
=20
commit 70bc49d421c793f73a772ae1f50622a39c6136d9
Date: Mon Feb 17 13:44:06 2014 -0800
=20
bcache: Fix another compiler warning on m68k
=20
Use a bigger hammer this time
=20
=20
diff --git a/drivers/md/bcache/bset.c b/drivers/md/bcache/bset.c
index 3f74b4b074..5454164153 100644
--- a/drivers/md/bcache/bset.c
+++ b/drivers/md/bcache/bset.c
@@ -23,8 +23,8 @@ void bch_dump_bset(struct btree_keys *b, struct bse=

t *i, unsigned set)

for (k =3D i->start; k < bset_bkey_last(i); k =3D next) {
next =3D bkey_next(k);
=20
- printk(KERN_ERR "block %u key %li/%u: ", set,
- (uint64_t *) k - i->d, i->keys);
+ printk(KERN_ERR "block %u key %u/%u: ", set,
+ (unsigned) ((u64 *) k - i->d), i->keys);
=20
if (b->ops->key_dump)
b->ops->key_dump(b, k);
=20

Could that cause a truncation? unsigned means unsigned int.
Can unsigned int be smaller (fewer bits) than the k pointer?
If so, is that OK or a problem?

--=20
~Randy

Kent Overstreet

2014-02-17 22:46:45 UTC

Post by Randy Dunlap
Could that cause a truncation? unsigned means unsigned int.
Can unsigned int be smaller (fewer bits) than the k pointer?
If so, is that OK or a problem?

It's just truncating the offset of the pointer within the struct bset, which is
part of a btree node - the thing that's being cast is restricted to be no bigger
than a btree node, which is at most a few mb.

Geert Uytterhoeven

2014-02-18 09:01:54 UTC