Discussion:
[PATCH 0/2] mtd-utils: Fix printf format specifier for off_t and loff_t
t***@t-online.de
2017-02-11 16:43:30 UTC
Permalink
From: Torsten Fleischer <***@t-online.de>

On different systems the size of off_t and loff_t can differ from each
other and they can have a size of either 4 or 8 byte.

The first patch creates the definitions for the printf format specifier
for both data types according to the size of them.
This fixes several compiler warnings on 32bit systems.

The second patch replaces the format specifier that is wrong for 32bit
systems by the appropriate definition for off_t.

Torsten Fleischer (2):
mtd-utils: Fix format specifier definitions for off_t and loff_t.
mtd-utils: serve_image: Use PRIdoff_t as format specifier.

configure.ac | 2 +-
include/common.h | 14 +++++++++-----
misc-utils/serve_image.c | 4 +++-
tests/mtd-tests/nandpagetest.c | 2 +-
4 files changed, 14 insertions(+), 8 deletions(-)
--
2.1.4
t***@t-online.de
2017-02-11 16:43:31 UTC
Permalink
From: Torsten Fleischer <***@t-online.de>

On 32bit systems (e.g. ARM) the size of off_t can be 4 byte and the size of loff_t 8 byte.

This causes compiler warnings like the following:

flash_erase.c: In function 'show_progress':
flash_erase.c:56:22: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 3 has type 'off_t {aka long int}' [-Wformat=]
bareverbose(!quiet, "\rErasing %d Kibyte @ %"PRIxoff_t" -- %2i %% complete ",

and an output like this:

~# flash_erase /dev/mtd2 0 1
Erasing 64 Kibyte @ 6400000000 -- 0 % complete

~#

Since the size of off_t and loff_t can differ from each other, the
printf format specifier should be determined separately for both.
Further the format specifiers should be based directly on the size of the
particular data type.

Signed-off-by: Torsten Fleischer <***@gmail.com>
---
configure.ac | 2 +-
include/common.h | 14 +++++++++-----
tests/mtd-tests/nandpagetest.c | 2 +-
3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index a5fc3a5..fbef15c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -149,7 +149,7 @@ AM_COND_IF([WITHOUT_LZO], [], [
test "${have_lzo}" != "yes" && AC_MSG_ERROR([lzo missing])
])

-AC_CHECK_SIZEOF([long])
+AC_CHECK_SIZEOF([off_t])

AC_CHECK_SIZEOF([loff_t])

diff --git a/include/common.h b/include/common.h
index 2812f49..d0c706d 100644
--- a/include/common.h
+++ b/include/common.h
@@ -70,17 +70,21 @@ extern "C" {
#endif

/* define a print format specifier for off_t */
-#ifdef __USE_FILE_OFFSET64
+#if (SIZEOF_OFF_T >= 8)
#define PRIxoff_t PRIx64
#define PRIdoff_t PRId64
#else
-#if (SIZEOF_LONG == SIZEOF_LOFF_T)
#define PRIxoff_t "l"PRIx32
#define PRIdoff_t "l"PRId32
-#else
-#define PRIxoff_t "ll"PRIx32
-#define PRIdoff_t "ll"PRId32
#endif
+
+/* define a print format specifier for loff_t */
+#if (SIZEOF_LOFF_T >= 8)
+#define PRIxloff_t PRIx64
+#define PRIdloff_t PRId64
+#else
+#define PRIxloff_t "l"PRIx32
+#define PRIdloff_t "l"PRId32
#endif

/* Verbose messages */
diff --git a/tests/mtd-tests/nandpagetest.c b/tests/mtd-tests/nandpagetest.c
index faf5fe3..4145ef7 100644
--- a/tests/mtd-tests/nandpagetest.c
+++ b/tests/mtd-tests/nandpagetest.c
@@ -230,7 +230,7 @@ static int verify_eraseblock(int ebnum)
return err;

if (lseek(fd, addr, SEEK_SET) != addr) {
- fprintf(stderr, "cannot seek mtd%d to offset %"PRIdoff_t,
+ fprintf(stderr, "cannot seek mtd%d to offset %"PRIdloff_t,
mtd.mtd_num, addr);
return -1;
}
--
2.1.4
t***@t-online.de
2017-02-11 16:43:32 UTC
Permalink
From: Torsten Fleischer <***@t-online.de>

To be independent on the size of off_t the format specifier determined of
common.h should be used instead of PRIu64.

Signed-off-by: Torsten Fleischer <***@gmail.com>
---
misc-utils/serve_image.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/misc-utils/serve_image.c b/misc-utils/serve_image.c
index cbcf3b7..26632e3 100644
--- a/misc-utils/serve_image.c
+++ b/misc-utils/serve_image.c
@@ -18,6 +18,8 @@
#include <crc32.h>
#include <inttypes.h>

+#include <common.h>
+
#include "mcast_image.h"

int tx_rate = 80000;
@@ -126,7 +128,7 @@ int main(int argc, char **argv)
}

if (st.st_size % erasesize) {
- fprintf(stderr, "Image size %" PRIu64 " bytes is not a multiple of erasesize %d bytes\n",
+ fprintf(stderr, "Image size %" PRIdoff_t " bytes is not a multiple of erasesize %d bytes\n",
st.st_size, erasesize);
exit(1);
}
--
2.1.4
David Oberhollenzer
2017-02-20 13:00:01 UTC
Permalink
Hi,

sorry for the delay, I just got around to testing this on a 32 bit
system today.

The patches look okay, but I wanted to at least try to reproduce
the warnings that the patch fixes.

I'm a bit cautious here, as this particular piece of code has been
patched repeatedly for this exact reason. Should there be further
problems, I guess at some point it would be easier to just remove
the macros and cast the printf arguments.


Applied to mtd-utils.git


Thanks,

David

Loading...