Page MenuHomeFreeBSD

Fix ggated/ggatec debug print of offsets.
ClosedPublic

Authored by ota_j.email.ne.jp on Aug 23 2019, 11:57 PM.

Details

Summary

Stop casting and use u/llu for printing unsigned 32bit/64 bit numbers.
The current format displays negative offsets while geom only deals with positive offset.

90 struct g_gate_hdr {
91 uint8_t gh_cmd; /* command */
92 uint64_t gh_offset; /* device offset */
93 uint32_t gh_length; /* size of block */
94 uint64_t gh_seq; /* request number */
95 uint16_t gh_error; /* error value (0 if ok) */
96 } __packed;

Test Plan

Run ggated/ggatec with -v and verify negative offset disappears with this change.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

As a matter of style we usually prefer to use %ju and a cast to uintmax_t instead of PRIu64.

I don't quite understand the change. off_t is indeed signed, so why do we want to hide negative values? They should result in errors anyway.

ggate defines its offset in unsigned int 64 as below.
So, printing negative numbers from printf indicates using a wrong formatter.

The PR formatter is to be architecture independent.
Is it better to use C99 provided formatter?
If not, we can use %u for unsigned 32bit int and %llu for unsigned 64bit, cannot we?

https://svnweb.freebsd.org/base/head/sbin/ggate/shared/ggate.h?revision=351936&view=markup#l90

90 struct g_gate_hdr {
91 uint8_t gh_cmd; /* command */
92 uint64_t gh_offset; /* device offset */
93 uint32_t gh_length; /* size of block */
94 uint64_t gh_seq; /* request number */
95 uint16_t gh_error; /* error value (0 if ok) */
96 } __packed;

Stop using PR* formatter.
Tested on i386 and amd64.

ggate defines its offset in unsigned int 64 as below.
So, printing negative numbers from printf indicates using a wrong formatter.

I'm sorry, I see now. For some reason I thought that r_offset is off_t, not uint64_t.

The PR formatter is to be architecture independent.
Is it better to use C99 provided formatter?
If not, we can use %u for unsigned 32bit int and %llu for unsigned 64bit, cannot we?

For printing off_t (i.e., a typedef), it is better to cast to intmax_t. For printing a raw uint64_t it is ok to use PR formatters. They are generally discouraged in FreeBSD, but I don't object to it. I do not think it is right to use %llu though.

So, either of the suggested approaches are ok with me. Sorry again for the confusion.

Thank you for checking this again.
It was also my bad that the problem wasn't described well enough.

For formatting, I'd use PR format as it is architecture independent.

I also changed "size" to "length" in ggatec.c. Ggated.c uses length and
also the field name is length.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 9 2020, 12:58 PM
This revision was automatically updated to reflect the committed changes.