Re: svn commit: r304221 - head/sys/boot/efi/boot1

[Emmanuel Vadot]

On Tue, 16 Aug 2016 12:16:30 -0600
Warner Losh  wrote:

> On Tue, Aug 16, 2016 at 8:57 AM, Ed Schouten  wrote:
> > Hi Emmanuel,
> >
> > 2016-08-16 16:23 GMT+02:00 Emmanuel Vadot :
> >> Author: manu
> >> Date: Tue Aug 16 14:23:35 2016
> >> New Revision: 304221
> >> URL: https://svnweb.freebsd.org/changeset/base/304221
> >>
> >> Log:
> >>   Use %ju modifier for u_int64_t and %jd modifier for off_t.
> >>   off_t is long long on arm32 and long on amd64
> >
> > I think both of these should be solved differently:
> >
> > - For uint64_t, you can use 's PRIu64 in the formatting
> > string. In kernel space, I suspect you need to use something like
> > .
> 
> cast it to intmax_t and use %jd. We've shunned PRIu64 in the tree.
> It's existing practice, but I'm sure bruce will voice mild distaste.
> 
> > - For off_t, it's all right to print it with %jd, but then be sure to
> > also add a cast to the argument itself. It may not necessarily be
> > equal to an intmax_t.
> 
> The cast is important.
> 
> Warner


 Thanks to all of you, I didn't know about PRI* (but I guess this was a
good thing to not know about them).

-- 
Emmanuel Vadot
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r304221 - head/sys/boot/efi/boot1

[Warner Losh]

On Tue, Aug 16, 2016 at 8:57 AM, Ed Schouten  wrote:
> Hi Emmanuel,
>
> 2016-08-16 16:23 GMT+02:00 Emmanuel Vadot :
>> Author: manu
>> Date: Tue Aug 16 14:23:35 2016
>> New Revision: 304221
>> URL: https://svnweb.freebsd.org/changeset/base/304221
>>
>> Log:
>>   Use %ju modifier for u_int64_t and %jd modifier for off_t.
>>   off_t is long long on arm32 and long on amd64
>
> I think both of these should be solved differently:
>
> - For uint64_t, you can use 's PRIu64 in the formatting
> string. In kernel space, I suspect you need to use something like
> .

cast it to intmax_t and use %jd. We've shunned PRIu64 in the tree.
It's existing practice, but I'm sure bruce will voice mild distaste.

> - For off_t, it's all right to print it with %jd, but then be sure to
> also add a cast to the argument itself. It may not necessarily be
> equal to an intmax_t.

The cast is important.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r304221 - head/sys/boot/efi/boot1

[Bruce Evans]

On Tue, 16 Aug 2016, Ed Schouten wrote:


Hi Emmanuel,


Log:
  Use %ju modifier for u_int64_t and %jd modifier for off_t.
  off_t is long long on arm32 and long on amd64


I think both of these should be solved differently:

- For uint64_t, you can use 's PRIu64 in the formatting
string. In kernel space, I suspect you need to use something like
.


Ugh.


- For off_t, it's all right to print it with %jd, but then be sure to
also add a cast to the argument itself. It may not necessarily be
equal to an intmax_t.


This shows how stupid the PRI* macros are.  They might be available for
0.1% of typedefed types in a medium-sized source tree.  But to use them,
you have to know their exact type, and change all printfs using them
whenever the typedef is changed.  If it is changed to a non-fixed width
type, then the printfs need lots of editing to change to a cast.  Their
only advantage is that they are more space and time efficient, especially
on 16-bit systems.

Extensive use of fixed-width type is another bug.  It asks for a fixed
ABI at any cost to efficiency or space.  FreeBSD almost never uses
"fast" or "least" integer types.  However, if you use these types, there
are PRI* mistakes for them too.

The SCN* macros are not quite as stupid as PRI*, but they should never
be used.  scanf() is already unusable since it gives undefined
behaviour on overflow.  These macros are not quite as stupid as PRI*
since casts don't work so well for input.  The corrsponding thing is
to scan input into variables of type [u]intmax_t and convert to the
corresponding type, of course without any bounds checking so that
you get similar undefined behaviour on overflow as when using SCN*.

Bruce
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r304221 - head/sys/boot/efi/boot1

[Ed Schouten]

Hi Emmanuel,

2016-08-16 16:23 GMT+02:00 Emmanuel Vadot :
> Author: manu
> Date: Tue Aug 16 14:23:35 2016
> New Revision: 304221
> URL: https://svnweb.freebsd.org/changeset/base/304221
>
> Log:
>   Use %ju modifier for u_int64_t and %jd modifier for off_t.
>   off_t is long long on arm32 and long on amd64

I think both of these should be solved differently:

- For uint64_t, you can use 's PRIu64 in the formatting
string. In kernel space, I suspect you need to use something like
.
- For off_t, it's all right to print it with %jd, but then be sure to
also add a cast to the argument itself. It may not necessarily be
equal to an intmax_t.

-- 
Ed Schouten 
Nuxi, 's-Hertogenbosch, the Netherlands
KvK-nr.: 62051717
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r304221 - head/sys/boot/efi/boot1

[Emmanuel Vadot]

Author: manu
Date: Tue Aug 16 14:23:35 2016
New Revision: 304221
URL: https://svnweb.freebsd.org/changeset/base/304221

Log:
  Use %ju modifier for u_int64_t and %jd modifier for off_t.
  off_t is long long on arm32 and long on amd64
  
  MFC after:1 week

Modified:
  head/sys/boot/efi/boot1/ufs_module.c
  head/sys/boot/efi/boot1/zfs_module.c

Modified: head/sys/boot/efi/boot1/ufs_module.c
==
--- head/sys/boot/efi/boot1/ufs_module.cTue Aug 16 14:15:09 2016
(r304220)
+++ head/sys/boot/efi/boot1/ufs_module.cTue Aug 16 14:23:35 2016
(r304221)
@@ -56,7 +56,7 @@ dskread(void *buf, u_int64_t lba, int nb
devinfo->dev->Media->MediaId, lba, size, buf);
 
if (status != EFI_SUCCESS) {
-   DPRINTF("dskread: failed dev: %p, id: %u, lba: %lu, size: %d, "
+   DPRINTF("dskread: failed dev: %p, id: %u, lba: %zu, size: %d, "
"status: %lu\n", devinfo->dev,
devinfo->dev->Media->MediaId, lba, size,
EFI_ERROR_CODE(status));

Modified: head/sys/boot/efi/boot1/zfs_module.c
==
--- head/sys/boot/efi/boot1/zfs_module.cTue Aug 16 14:15:09 2016
(r304220)
+++ head/sys/boot/efi/boot1/zfs_module.cTue Aug 16 14:23:35 2016
(r304221)
@@ -53,7 +53,7 @@ vdev_read(vdev_t *vdev, void *priv, off_
status = devinfo->dev->ReadBlocks(devinfo->dev,
devinfo->dev->Media->MediaId, lba, bytes, buf);
if (status != EFI_SUCCESS) {
-   DPRINTF("vdev_read: failed dev: %p, id: %u, lba: %zu, size: 
%zu,"
+   DPRINTF("vdev_read: failed dev: %p, id: %u, lba: %jd, size: 
%zu,"
 " status: %lu\n", devinfo->dev,
 devinfo->dev->Media->MediaId, lba, bytes,
 EFI_ERROR_CODE(status));
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"