[PATCHv1 1/5] mkfs.jffs2: fix casting of printf argument

[PATCHv1 1/5] mkfs.jffs2: fix casting of printf argument

[Andy Shevchenko]

The compiler warns us about cast mismatch for %9lu specifier.  In original code
the argument has __off64_t type. Here is a simple type casting fix.

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 mkfs.jffs2.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/mkfs.jffs2.c b/mkfs.jffs2.c
index 3aab533..7bb9ad1 100644
--- a/mkfs.jffs2.c
+++ b/mkfs.jffs2.c
@@ -1233,7 +1233,7 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 			case S_IFDIR:
 				if (verbose) {
 					printf("\td %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
+							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
 							(int) (e->sb.st_uid), (int) (e->sb.st_gid),
 							e->name);
 				}
@@ -1243,7 +1243,7 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 			case S_IFSOCK:
 				if (verbose) {
 					printf("\ts %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
+							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				write_pipe(e);
@@ -1252,7 +1252,7 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 			case S_IFIFO:
 				if (verbose) {
 					printf("\tp %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
+							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				write_pipe(e);
@@ -1281,7 +1281,7 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 			case S_IFLNK:
 				if (verbose) {
 					printf("\tl %04o %9lu             %5d:%-3d %s -> %s\n",
-							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
+							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name,
 							e->link);
 				}
@@ -1293,8 +1293,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				write_xattr_entry(e);
 				if (verbose) {
 					printf("\tf %04o %9lu (%9u) %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, e->sb.st_size, wrote,
-							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
+							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+							wrote, (int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				break;
 			default:
-- 
1.6.3.3

[PATCHv1 2/5] serve_image: adjust type for printf

[Andy Shevchenko]

The argument type of printf is __off64_t, meanwhile the classificator is "%ld".

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 serve_image.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/serve_image.c b/serve_image.c
index f8f28a1..9eaf9dd 100644
--- a/serve_image.c
+++ b/serve_image.c
@@ -127,7 +127,7 @@ int main(int argc, char **argv)
 
 	if (st.st_size % erasesize) {
 		fprintf(stderr, "Image size %ld bytes is not a multiple of erasesize %d bytes\n",
-			st.st_size, erasesize);
+			(long)st.st_size, erasesize);
 		exit(1);
 	}
 	image = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, rfd, 0);
-- 
1.6.3.3

Re: [PATCHv1 2/5] serve_image: adjust type for printf

[Artem Bityutskiy]

On Wed, 2011-04-13 at 16:20 +0300, Andy Shevchenko wrote:
>  	if (st.st_size % erasesize) {
>  		fprintf(stderr, "Image size %ld bytes is not a multiple of erasesize %d bytes\n",
> -			st.st_size, erasesize);
> +			(long)st.st_size, erasesize);

Same here.


-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

[PATCHv1 3/5] tests: fs-tests: read() returns ssize_t value

[Andy Shevchenko]

Use ssize_t instead of size_t.

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 tests/fs-tests/stress/atoms/rndwrite00.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/fs-tests/stress/atoms/rndwrite00.c b/tests/fs-tests/stress/atoms/rndwrite00.c
index 655d9cc..3c38e43 100644
--- a/tests/fs-tests/stress/atoms/rndwrite00.c
+++ b/tests/fs-tests/stress/atoms/rndwrite00.c
@@ -36,7 +36,7 @@
 
 static void check_file(int fd, char *data, size_t length)
 {
-	size_t n, i;
+	ssize_t n, i;
 	char buf[BUFFER_SIZE];
 
 	CHECK(lseek(fd, 0, SEEK_SET) != -1);
-- 
1.6.3.3

Re: [PATCHv1 3/5] tests: fs-tests: read() returns ssize_t value

[Artem Bityutskiy]

On Wed, 2011-04-13 at 16:20 +0300, Andy Shevchenko wrote:
> Use ssize_t instead of size_t.
> 
> Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>

Pushed this one, thanks!

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

[PATCHv1 4/5] tests: checkfs: adjust Makefile

[Andy Shevchenko]

This patch brings common Makefile (in terms of mtd-utils project) to the
checkfs test suite. Additionally it fixes a build error related to usage of
open().

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 tests/checkfs/Makefile  |   14 +++-----------
 tests/checkfs/checkfs.c |    2 +-
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/tests/checkfs/Makefile b/tests/checkfs/Makefile
index ac94dde..0a77682 100644
--- a/tests/checkfs/Makefile
+++ b/tests/checkfs/Makefile
@@ -1,14 +1,6 @@
+TARGETS = checkfs makefiles
 
-all: checkfs makefiles
+include ../../common.mk
 
-checkfs: checkfs.c Makefile common.h comm.o
-	gcc -g -Wall checkfs.c comm.o -o checkfs
+$(TARGETS): $(addprefix $(BUILDDIR)/, comm.o)
 
-comm.o: comm.c Makefile
-	gcc -g -Wall -c comm.c -o comm.o
-
-makefiles: makefiles.c Makefile common.h
-	gcc -g -Wall makefiles.c -o makefiles
-
-clean:
-	rm -f makefiles checkfs *~ *.o
diff --git a/tests/checkfs/checkfs.c b/tests/checkfs/checkfs.c
index 3224d2b..8e74da8 100644
--- a/tests/checkfs/checkfs.c
+++ b/tests/checkfs/checkfs.c
@@ -414,7 +414,7 @@ void make_new_file(char *filename){
 
     fprintf(stderr, "Creating File:%s. ", filename);
 
-    if((dfd = open(filename, O_RDWR | O_CREAT | O_SYNC)) <= 0)
+    if((dfd = open(filename, O_RDWR | O_CREAT | O_SYNC, S_IRWXU)) <= 0)
     {
         printf("Error! Cannot open file: %s\n",filename);
         perror("Error");
-- 
1.6.3.3

Re: [PATCHv1 4/5] tests: checkfs: adjust Makefile

[Artem Bityutskiy]

On Wed, 2011-04-13 at 16:20 +0300, Andy Shevchenko wrote:
> This patch brings common Makefile (in terms of mtd-utils project) to the
> checkfs test suite. Additionally it fixes a build error related to usage of
> open().
> 
> Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
> ---
>  tests/checkfs/Makefile  |   14 +++-----------
>  tests/checkfs/checkfs.c |    2 +-
>  2 files changed, 4 insertions(+), 12 deletions(-)

You should not mix makefile and code changes. But well, not so big deal
in this case, pushed, thanks!

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

[PATCHv1 5/5] tests: ubi-tests: clean libubi.a and *.o

[Andy Shevchenko]

The common.mk contains clean target that removes *.o and $(TARGETS). Thus, make
custom clean target only for libubi.a

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 tests/ubi-tests/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/ubi-tests/Makefile b/tests/ubi-tests/Makefile
index d6be0dc..741a5c2 100644
--- a/tests/ubi-tests/Makefile
+++ b/tests/ubi-tests/Makefile
@@ -22,4 +22,4 @@ libubi.a: $(LIBUBI_SRC_PATH)/libubi.c  $(LIBUBI_HEADER_PATH)/libubi.h  $(LIBUBI_
 $(TARGETS): $(addprefix $(BUILDDIR)/, common.o) libubi.a
 
 clean::
-	rm -f $(TARGETS) $(addsuffix .o, $(TESTS)) libubi.*
+	rm -f libubi.a
-- 
1.6.3.3

Re: [PATCHv1 5/5] tests: ubi-tests: clean libubi.a and *.o

[Artem Bityutskiy]

On Wed, 2011-04-13 at 16:20 +0300, Andy Shevchenko wrote:
> The common.mk contains clean target that removes *.o and $(TARGETS). Thus, make
> custom clean target only for libubi.a
> 
> Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>

Pushed, thanks.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCHv1 1/5] mkfs.jffs2: fix casting of printf argument

[Artem Bityutskiy]

On Wed, 2011-04-13 at 16:20 +0300, Andy Shevchenko wrote:
> The compiler warns us about cast mismatch for %9lu specifier.  In original code
> the argument has __off64_t type. Here is a simple type casting fix.
> 
> Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>

If it is off64_t then you should better change the printf() placeholder
to %ull and cast the argument to (unsigned long long)

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCHv1 1/5] mkfs.jffs2: fix casting of printf argument

[Andy Shevchenko]

On Thu, 2011-04-14 at 15:24 +0300, Artem Bityutskiy wrote:
> On Wed, 2011-04-13 at 16:20 +0300, Andy Shevchenko wrote:
> > The compiler warns us about cast mismatch for %9lu specifier.  In original code
> > the argument has __off64_t type. Here is a simple type casting fix.
> > 
> > Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
> 
> If it is off64_t then you should better change the printf() placeholder
> to %ull and cast the argument to (unsigned long long)
> 
I was waiting for such correction because I don't know the limits of the
argument, thanks.


-- 
With Best Regards,
Andy Shevchenko

[PATCHv2 1/2] serve_image: adjust classificator and type for printf

[Andy Shevchenko]

The argument type of printf is __off64_t, meanwhile the classificator is "%ld".

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 serve_image.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/serve_image.c b/serve_image.c
index f8f28a1..d86e372 100644
--- a/serve_image.c
+++ b/serve_image.c
@@ -126,8 +126,9 @@ int main(int argc, char **argv)
 	}
 
 	if (st.st_size % erasesize) {
-		fprintf(stderr, "Image size %ld bytes is not a multiple of erasesize %d bytes\n",
-			st.st_size, erasesize);
+		fprintf(stderr, "Image size %llu bytes is not a multiple of "
+				"erasesize %d bytes\n",
+				(unsigned long long)st.st_size, erasesize);
 		exit(1);
 	}
 	image = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, rfd, 0);
-- 
1.6.3.3

[PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Andy Shevchenko]

The casting of __off64_t to unsigned long potentially wrong for values higher
than ULONG_MAX.  Let's fix that.

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 mkfs.jffs2.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/mkfs.jffs2.c b/mkfs.jffs2.c
index 7bb9ad1..01db4be 100644
--- a/mkfs.jffs2.c
+++ b/mkfs.jffs2.c
@@ -1232,8 +1232,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 		} else switch (e->sb.st_mode & S_IFMT) {
 			case S_IFDIR:
 				if (verbose) {
-					printf("\td %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\td %04o %llu             %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, (unsigned long long) e->sb.st_size,
 							(int) (e->sb.st_uid), (int) (e->sb.st_gid),
 							e->name);
 				}
@@ -1242,8 +1242,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				break;
 			case S_IFSOCK:
 				if (verbose) {
-					printf("\ts %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\ts %04o %llu             %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, (unsigned long long) e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				write_pipe(e);
@@ -1251,8 +1251,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				break;
 			case S_IFIFO:
 				if (verbose) {
-					printf("\tp %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\tp %04o %llu             %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, (unsigned long long) e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				write_pipe(e);
@@ -1280,8 +1280,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				break;
 			case S_IFLNK:
 				if (verbose) {
-					printf("\tl %04o %9lu             %5d:%-3d %s -> %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\tl %04o %llu             %5d:%-3d %s -> %s\n",
+							e->sb.st_mode & ~S_IFMT, (unsigned long long) e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name,
 							e->link);
 				}
@@ -1292,8 +1292,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				wrote = write_regular_file(e);
 				write_xattr_entry(e);
 				if (verbose) {
-					printf("\tf %04o %9lu (%9u) %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\tf %04o %llu (%9u) %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, (unsigned long long) e->sb.st_size,
 							wrote, (int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				break;
-- 
1.6.3.3

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Mike Frysinger]

On Mon, Apr 18, 2011 at 04:31, Andy Shevchenko wrote:
> The casting of __off64_t to unsigned long potentially wrong for values higher
> than ULONG_MAX.  Let's fix that.

i dont think this is the way to go.  on 64bit systems, long long is
128bits.  i imagine the way to go (assuming we're always using LFS) is
to use PRIu64 from inttypes.h

printf("\td %04o %9" PRIu64 "             %5d:%-3d %s\n",
    e->sb.st_mode & ~S_IFMT, (unsigned long long) e->sb.st_size,
    (int) (e->sb.st_uid), (int) (e->sb.st_gid),
    e->name);
-mike

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Artem Bityutskiy]

On Mon, 2011-04-18 at 09:49 -0400, Mike Frysinger wrote:
> On Mon, Apr 18, 2011 at 04:31, Andy Shevchenko wrote:
> > The casting of __off64_t to unsigned long potentially wrong for values higher
> > than ULONG_MAX.  Let's fix that.
> 
> i dont think this is the way to go.  on 64bit systems, long long is
> 128bits.  i imagine the way to go (assuming we're always using LFS) is
> to use PRIu64 from inttypes.h

sizeof(unsigned long long) is 8 (64 bits) on my x86_64 fedora.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Mike Frysinger]

On Mon, Apr 18, 2011 at 09:55, Artem Bityutskiy wrote:
> On Mon, 2011-04-18 at 09:49 -0400, Mike Frysinger wrote:
>> On Mon, Apr 18, 2011 at 04:31, Andy Shevchenko wrote:
>> > The casting of __off64_t to unsigned long potentially wrong for values higher
>> > than ULONG_MAX.  Let's fix that.
>>
>> i dont think this is the way to go.  on 64bit systems, long long is
>> 128bits.  i imagine the way to go (assuming we're always using LFS) is
>> to use PRIu64 from inttypes.h
>
> sizeof(unsigned long long) is 8 (64 bits) on my x86_64 fedora.

so it is.  i still think PRIu64 is the correct way to handle this as
there is no sizeof() assumption and no need for casting.
-mike

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Artem Bityutskiy]

On Mon, 2011-04-18 at 10:04 -0400, Mike Frysinger wrote:
> On Mon, Apr 18, 2011 at 09:55, Artem Bityutskiy wrote:
> > On Mon, 2011-04-18 at 09:49 -0400, Mike Frysinger wrote:
> >> On Mon, Apr 18, 2011 at 04:31, Andy Shevchenko wrote:
> >> > The casting of __off64_t to unsigned long potentially wrong for values higher
> >> > than ULONG_MAX.  Let's fix that.
> >>
> >> i dont think this is the way to go.  on 64bit systems, long long is
> >> 128bits.  i imagine the way to go (assuming we're always using LFS) is
> >> to use PRIu64 from inttypes.h
> >
> > sizeof(unsigned long long) is 8 (64 bits) on my x86_64 fedora.
> 
> so it is.  i still think PRIu64 is the correct way to handle this as
> there is no sizeof() assumption and no need for casting.

Never used this, but yes, as long as this is something which has worked
for ages and we are not going to have "this is not supported" issues -
sure!

But unsigned long long is 64 bits I think in all GNU systems, and
casting to unsigned long long is quite standard practice AFAIK, so I do
not see why it would be very bad thing to do.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Mike Frysinger]

On Mon, Apr 18, 2011 at 10:07, Artem Bityutskiy wrote:
> On Mon, 2011-04-18 at 10:04 -0400, Mike Frysinger wrote:
>> On Mon, Apr 18, 2011 at 09:55, Artem Bityutskiy wrote:
>> > On Mon, 2011-04-18 at 09:49 -0400, Mike Frysinger wrote:
>> >> On Mon, Apr 18, 2011 at 04:31, Andy Shevchenko wrote:
>> >> > The casting of __off64_t to unsigned long potentially wrong for values higher
>> >> > than ULONG_MAX.  Let's fix that.
>> >>
>> >> i dont think this is the way to go.  on 64bit systems, long long is
>> >> 128bits.  i imagine the way to go (assuming we're always using LFS) is
>> >> to use PRIu64 from inttypes.h
>> >
>> > sizeof(unsigned long long) is 8 (64 bits) on my x86_64 fedora.
>>
>> so it is.  i still think PRIu64 is the correct way to handle this as
>> there is no sizeof() assumption and no need for casting.
>
> Never used this, but yes, as long as this is something which has worked
> for ages and we are not going to have "this is not supported" issues -
> sure!

glibc has been shipping this since 1999 (if not earlier), so i dont
think that'll be an issue.  glibc-2.2.5 was released in 2002, and it's
hard to even find that anymore.

> But unsigned long long is 64 bits I think in all GNU systems, and
> casting to unsigned long long is quite standard practice AFAIK, so I do
> not see why it would be very bad thing to do.

i think we should strive for the correct types in the printf string
rather than attempting to cast things away all the time.  casting
things away can make us miss stuff, such as printf'ing a 64bit as a
32bit ;).
-mike

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Artem Bityutskiy]

On Mon, 2011-04-18 at 10:21 -0400, Mike Frysinger wrote:
> On Mon, Apr 18, 2011 at 10:07, Artem Bityutskiy wrote:
> > On Mon, 2011-04-18 at 10:04 -0400, Mike Frysinger wrote:
> >> On Mon, Apr 18, 2011 at 09:55, Artem Bityutskiy wrote:
> >> > On Mon, 2011-04-18 at 09:49 -0400, Mike Frysinger wrote:
> >> >> On Mon, Apr 18, 2011 at 04:31, Andy Shevchenko wrote:
> >> >> > The casting of __off64_t to unsigned long potentially wrong for values higher
> >> >> > than ULONG_MAX.  Let's fix that.
> >> >>
> >> >> i dont think this is the way to go.  on 64bit systems, long long is
> >> >> 128bits.  i imagine the way to go (assuming we're always using LFS) is
> >> >> to use PRIu64 from inttypes.h
> >> >
> >> > sizeof(unsigned long long) is 8 (64 bits) on my x86_64 fedora.
> >>
> >> so it is.  i still think PRIu64 is the correct way to handle this as
> >> there is no sizeof() assumption and no need for casting.
> >
> > Never used this, but yes, as long as this is something which has worked
> > for ages and we are not going to have "this is not supported" issues -
> > sure!
> 
> glibc has been shipping this since 1999 (if not earlier), so i dont
> think that'll be an issue.  glibc-2.2.5 was released in 2002, and it's
> hard to even find that anymore.
> 

OK, sure, thanks for pointing this!

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Andy Shevchenko]

On Mon, 2011-04-18 at 17:07 +0300, ext Artem Bityutskiy wrote:
> On Mon, 2011-04-18 at 10:04 -0400, Mike Frysinger wrote:
> > On Mon, Apr 18, 2011 at 09:55, Artem Bityutskiy wrote:
> > > On Mon, 2011-04-18 at 09:49 -0400, Mike Frysinger wrote:
> > >> On Mon, Apr 18, 2011 at 04:31, Andy Shevchenko wrote:
> > >> > The casting of __off64_t to unsigned long potentially wrong for values higher
> > >> > than ULONG_MAX.  Let's fix that.
> > >>
> > >> i dont think this is the way to go.  on 64bit systems, long long is
> > >> 128bits.  i imagine the way to go (assuming we're always using LFS) is
> > >> to use PRIu64 from inttypes.h
> > >
> > > sizeof(unsigned long long) is 8 (64 bits) on my x86_64 fedora.
> > 
> > so it is.  i still think PRIu64 is the correct way to handle this as
> > there is no sizeof() assumption and no need for casting.
> 
> Never used this, but yes, as long as this is something which has worked
> for ages and we are not going to have "this is not supported" issues -
> sure!
This is described in C99 ISO standard [1].

> But unsigned long long is 64 bits I think in all GNU systems, and
> casting to unsigned long long is quite standard practice AFAIK, so I do
> not see why it would be very bad thing to do.
Yeah, some projects use following approach
#ifndef PRIu64
#define PRIu64 "llu"
#endif

The opposite recommendation is to put "%9jd" classifier to print st_size field of struct stat [2].

[1]
http://pubs.opengroup.org/onlinepubs/000095399/basedefs/inttypes.h.html
[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/printf.html


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Mike Frysinger]

On Mon, Apr 18, 2011 at 10:26, Andy Shevchenko wrote:
> [1]
> http://pubs.opengroup.org/onlinepubs/000095399/basedefs/inttypes.h.html
> [2] http://pubs.opengroup.org/onlinepubs/009695399/functions/printf.html

on the off chance you missed it, Issue 7 is out:
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/inttypes.h.html
-mike

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Andy Shevchenko]

On Mon, 2011-04-18 at 10:29 -0400, ext Mike Frysinger wrote:
> On Mon, Apr 18, 2011 at 10:26, Andy Shevchenko wrote:
> > [1]
> > http://pubs.opengroup.org/onlinepubs/000095399/basedefs/inttypes.h.html
> > [2] http://pubs.opengroup.org/onlinepubs/009695399/functions/printf.html
> 
> on the off chance you missed it, Issue 7 is out:
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/inttypes.h.html
> -mike
It's true. However it doesn't matter to our talk.
The question is which way is better PRIu64 or %jd.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Mike Frysinger]

On Mon, Apr 18, 2011 at 10:50, Andy Shevchenko wrote:
> On Mon, 2011-04-18 at 10:29 -0400, ext Mike Frysinger wrote:
>> On Mon, Apr 18, 2011 at 10:26, Andy Shevchenko wrote:
>> > [1]
>> > http://pubs.opengroup.org/onlinepubs/000095399/basedefs/inttypes.h.html
>> > [2] http://pubs.opengroup.org/onlinepubs/009695399/functions/printf.html
>>
>> on the off chance you missed it, Issue 7 is out:
>> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/inttypes.h.html
>
> It's true. However it doesn't matter to our talk.

it doesnt ... i was just letting you know in case you werent aware.
Issue 7 has a lot of fun new features.

> The question is which way is better PRIu64 or %jd.

%jd means [u]intmax_t which is not what this is ... on a 32bit system,
int is 32bit.  we're dealing with an explicitly 64bit type (off64_t),
so let's go with an explicitly 64bit printf modifier.
-mike

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Andy Shevchenko]

On Mon, 2011-04-18 at 10:57 -0400, ext Mike Frysinger wrote:
> On Mon, Apr 18, 2011 at 10:50, Andy Shevchenko wrote:
> > On Mon, 2011-04-18 at 10:29 -0400, ext Mike Frysinger wrote:
> >> On Mon, Apr 18, 2011 at 10:26, Andy Shevchenko wrote:
> >> > [1]
> >> > http://pubs.opengroup.org/onlinepubs/000095399/basedefs/inttypes.h.html
> >> > [2] http://pubs.opengroup.org/onlinepubs/009695399/functions/printf.html
> >>
> >> on the off chance you missed it, Issue 7 is out:
> >> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/inttypes.h.html
> >
> > It's true. However it doesn't matter to our talk.
> 
> it doesnt ... i was just letting you know in case you werent aware.
> Issue 7 has a lot of fun new features.
> 
> > The question is which way is better PRIu64 or %jd.
> 
> %jd means [u]intmax_t which is not what this is ... on a 32bit system,
> int is 32bit.  we're dealing with an explicitly 64bit type (off64_t),
Accordingly to [1]

Limits of greatest-width integer types
      * Minimum value of greatest-width signed integer type:
        {INTMAX_MIN} -(2 63 -1)
      * Maximum value of greatest-width signed integer type:
        {INTMAX_MAX}  2 63 -1
      * Maximum value of greatest-width unsigned integer type:
        {UINTMAX_MAX} 2 64 -1

So, it fits to 64bit types.

> so let's go with an explicitly 64bit printf modifier.
Regarding to above I couldn't see any objection against %jd. At least
one for PRIu64 - it looks awful in code.


[1]
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdint.h.html


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Mike Frysinger]

On Mon, Apr 18, 2011 at 11:20, Andy Shevchenko wrote:
> On Mon, 2011-04-18 at 10:57 -0400, ext Mike Frysinger wrote:
>> so let's go with an explicitly 64bit printf modifier.
>
> Regarding to above I couldn't see any objection against %jd. At least
> one for PRIu64 - it looks awful in code.

it doesnt make sense to me to pair a non-explicit sized format string
with an explicit sized type.  off64_t is exactly 64bits, and PRIu64
prints exactly 64bits.

that said, %ju is at least an improvement over the current code as
well as the patch you've posted (doesnt seem to need casts on 32 or 64
bit systems).  so if Artem is OK with that, i wont fight it.  even
though it's still not as correct as my proposal :P.
-mike

Re: [PATCHv2 2/2] mkfs.jffs2: fix casting of __off64_t

[Artem Bityutskiy]

On Mon, 2011-04-18 at 17:33 -0400, Mike Frysinger wrote:
> On Mon, Apr 18, 2011 at 11:20, Andy Shevchenko wrote:
> > On Mon, 2011-04-18 at 10:57 -0400, ext Mike Frysinger wrote:
> >> so let's go with an explicitly 64bit printf modifier.
> >
> > Regarding to above I couldn't see any objection against %jd. At least
> > one for PRIu64 - it looks awful in code.
> 
> it doesnt make sense to me to pair a non-explicit sized format string
> with an explicit sized type.  off64_t is exactly 64bits, and PRIu64
> prints exactly 64bits.

Yes, I agree.

> that said, %ju is at least an improvement over the current code as
> well as the patch you've posted (doesnt seem to need casts on 32 or 64
> bit systems).  so if Artem is OK with that, i wont fight it.  even
> though it's still not as correct as my proposal :P.

To be frank, I do not care much :-) We are talking about a single place.
If Andy make a fix for all the mtd code, that would matter much much
more :-)

But yes, of course it is difficult to disagree that for explicitly sized
type it is saner to use explicit-size print placeholder.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

[PATCHv3 1/3] mkfs.jffs2: fix casting of __off64_t

[Andy Shevchenko]

The casting of __off64_t to unsigned long potentially wrong for values higher
than ULONG_MAX.  Let's fix that by using PRIu64 classifier.

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 mkfs.jffs2.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/mkfs.jffs2.c b/mkfs.jffs2.c
index 7bb9ad1..468dfe5 100644
--- a/mkfs.jffs2.c
+++ b/mkfs.jffs2.c
@@ -72,8 +72,9 @@
 #endif
 #include <byteswap.h>
 #include <crc32.h>
-#include "rbtree.h"
+#include <inttypes.h>
 
+#include "rbtree.h"
 #include "common.h"
 
 /* Do not use the weird XPG version of basename */
@@ -1232,8 +1233,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 		} else switch (e->sb.st_mode & S_IFMT) {
 			case S_IFDIR:
 				if (verbose) {
-					printf("\td %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\td %04o %" PRIu64 "             %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
 							(int) (e->sb.st_uid), (int) (e->sb.st_gid),
 							e->name);
 				}
@@ -1242,8 +1243,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				break;
 			case S_IFSOCK:
 				if (verbose) {
-					printf("\ts %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\ts %04o %" PRIu64 "             %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				write_pipe(e);
@@ -1251,8 +1252,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				break;
 			case S_IFIFO:
 				if (verbose) {
-					printf("\tp %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\tp %04o %" PRIu64 "             %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				write_pipe(e);
@@ -1280,8 +1281,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				break;
 			case S_IFLNK:
 				if (verbose) {
-					printf("\tl %04o %9lu             %5d:%-3d %s -> %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\tl %04o %" PRIu64 "             %5d:%-3d %s -> %s\n",
+							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name,
 							e->link);
 				}
@@ -1292,9 +1293,9 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				wrote = write_regular_file(e);
 				write_xattr_entry(e);
 				if (verbose) {
-					printf("\tf %04o %9lu (%9u) %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
-							wrote, (int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
+					printf("\tf %04o %" PRIu64 " (%9u) %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, e->sb.st_size, wrote,
+							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				break;
 			default:
-- 
1.6.3.3

[PATCHv3 2/3] libmtd: use PRIu64 classifier for uint64_t printf arguments

[Andy Shevchenko]

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 lib/libmtd.c |   30 ++++++++++++++----------------
 1 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/lib/libmtd.c b/lib/libmtd.c
index 418ba29..6901a27 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -31,9 +31,11 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/ioctl.h>
-#include <mtd/mtd-user.h>
+#include <inttypes.h>
 
+#include <mtd/mtd-user.h>
 #include <libmtd.h>
+
 #include "libmtd_int.h"
 #include "common.h"
 
@@ -1096,19 +1098,17 @@ int do_oob_op(libmtd_t desc, const struct mtd_dev_info *mtd, int fd,
 
 	max_offs = (unsigned long long)mtd->eb_cnt * mtd->eb_size;
 	if (start >= max_offs) {
-		errmsg("bad page address %llu, mtd%d has %d eraseblocks "
-		       "(%llu bytes)", (unsigned long long) start, mtd->mtd_num,
-		       mtd->eb_cnt, max_offs);
+		errmsg("bad page address %" PRIu64 ", mtd%d has %d eraseblocks"
+		       " (%llu bytes)", start, mtd->mtd_num, mtd->eb_cnt, max_offs);
 		errno = EINVAL;
 		return -1;
 	}
 
 	oob_offs = start & (mtd->min_io_size - 1);
 	if (oob_offs + length > mtd->oob_size || length == 0) {
-		errmsg("Cannot write %llu OOB bytes to address %llu "
-		       "(OOB offset %u) - mtd%d OOB size is only %d bytes",
-		       (unsigned long long)length, (unsigned long long)start,
-		       oob_offs, mtd->mtd_num,  mtd->oob_size);
+		errmsg("Cannot write %" PRIu64 " OOB bytes to address %" PRIu64
+		       " (OOB offset %u) - mtd%d OOB size is only %d bytes",
+		       length, start, oob_offs, mtd->mtd_num,  mtd->oob_size);
 		errno = EINVAL;
 		return -1;
 	}
@@ -1125,10 +1125,9 @@ int do_oob_op(libmtd_t desc, const struct mtd_dev_info *mtd, int fd,
 
 		if (errno != ENOTTY ||
 		    lib->offs64_ioctls != OFFS64_IOCTLS_UNKNOWN) {
-			sys_errmsg("%s ioctl failed for mtd%d, offset %llu "
-				   "(eraseblock %llu)", cmd64_str, mtd->mtd_num,
-				   (unsigned long long)start,
-				   (unsigned long long)start / mtd->eb_size);
+			sys_errmsg("%s ioctl failed for mtd%d, offset %" PRIu64
+				   " (eraseblock %" PRIu64 ")", cmd64_str,
+				   mtd->mtd_num, start, start / mtd->eb_size);
 		}
 
 		/*
@@ -1152,10 +1151,9 @@ int do_oob_op(libmtd_t desc, const struct mtd_dev_info *mtd, int fd,
 
 	ret = ioctl(fd, cmd, &oob);
 	if (ret < 0)
-		sys_errmsg("%s ioctl failed for mtd%d, offset %llu "
-			   "(eraseblock %llu)", cmd_str, mtd->mtd_num,
-			   (unsigned long long)start,
-			   (unsigned long long)start / mtd->eb_size);
+		sys_errmsg("%s ioctl failed for mtd%d, offset %" PRIu64
+			   " (eraseblock %" PRIu64 ")", cmd_str, mtd->mtd_num,
+			   start, start / mtd->eb_size);
 	return ret;
 }
 
-- 
1.6.3.3

[PATCHv3 3/3] serve_image: adjust classifier and type for printf

[Andy Shevchenko]

The argument type of printf is __off64_t, meanwhile the classifier is "%ld".
We agreed to use PRIu64 in such case.

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 serve_image.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/serve_image.c b/serve_image.c
index f8f28a1..0e7980f 100644
--- a/serve_image.c
+++ b/serve_image.c
@@ -2,7 +2,6 @@
 #define _POSIX_C_SOURCE 199309
 
 #include <time.h>
-
 #include <errno.h>  	
 #include <error.h> 	
 #include <netdb.h> 	
@@ -18,6 +17,8 @@
 #include <netinet/in.h>
 #include <sys/time.h>
 #include <crc32.h>
+#include <inttypes.h>
+
 #include "mcast_image.h"
 
 int tx_rate = 80000;
@@ -126,8 +127,8 @@ int main(int argc, char **argv)
 	}
 
 	if (st.st_size % erasesize) {
-		fprintf(stderr, "Image size %ld bytes is not a multiple of erasesize %d bytes\n",
-			st.st_size, erasesize);
+		fprintf(stderr, "Image size %" PRIu64 " bytes is not a multiple of "
+				"erasesize %d bytes\n", st.st_size, erasesize);
 		exit(1);
 	}
 	image = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, rfd, 0);
-- 
1.6.3.3

Re: [PATCHv3 3/3] serve_image: adjust classifier and type for printf

[Mike Frysinger]

On Tue, Apr 19, 2011 at 04:34, Andy Shevchenko wrote:
> -               fprintf(stderr, "Image size %ld bytes is not a multiple of erasesize %d bytes\n",
> -                       st.st_size, erasesize);
> +               fprintf(stderr, "Image size %" PRIu64 " bytes is not a multiple of "
> +                               "erasesize %d bytes\n", st.st_size, erasesize);

i think we want to avoid splitting string constants
-mike

Re: [PATCHv3 3/3] serve_image: adjust classifier and type for printf

[Andy Shevchenko]

On Tue, 2011-04-19 at 10:26 -0400, ext Mike Frysinger wrote:
> On Tue, Apr 19, 2011 at 04:34, Andy Shevchenko wrote:
> > -               fprintf(stderr, "Image size %ld bytes is not a multiple of erasesize %d bytes\n",
> > -                       st.st_size, erasesize);
> > +               fprintf(stderr, "Image size %" PRIu64 " bytes is not a multiple of "
> > +                               "erasesize %d bytes\n", st.st_size, erasesize);
> 
> i think we want to avoid splitting string constants
I looked to many other places in the code where it has split in string
constants. So, this is an irrelevant issue.
However, I could join back the strings which I touch in patches.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCHv3 3/3] serve_image: adjust classifier and type for printf

[Mike Frysinger]

On Tue, Apr 19, 2011 at 10:32, Andy Shevchenko wrote:
> On Tue, 2011-04-19 at 10:26 -0400, ext Mike Frysinger wrote:
>> On Tue, Apr 19, 2011 at 04:34, Andy Shevchenko wrote:
>> > -               fprintf(stderr, "Image size %ld bytes is not a multiple of erasesize %d bytes\n",
>> > -                       st.st_size, erasesize);
>> > +               fprintf(stderr, "Image size %" PRIu64 " bytes is not a multiple of "
>> > +                               "erasesize %d bytes\n", st.st_size, erasesize);
>>
>> i think we want to avoid splitting string constants
>
> I looked to many other places in the code where it has split in string
> constants. So, this is an irrelevant issue.
> However, I could join back the strings which I touch in patches.

the current code base has a lot of ugly code.  but that doesnt mean we
should introduce more, and since you're changing things, it doesnt
make it an irrelevant issue.
-mike

[PATCHv4 1/3] mkfs.jffs2: fix casting of __off64_t

[Andy Shevchenko]

The casting of __off64_t to unsigned long potentially wrong for values higher
than ULONG_MAX.  Let's fix that by using PRIu64 classifier.

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 mkfs.jffs2.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/mkfs.jffs2.c b/mkfs.jffs2.c
index 7bb9ad1..7abaf50 100644
--- a/mkfs.jffs2.c
+++ b/mkfs.jffs2.c
@@ -72,8 +72,9 @@
 #endif
 #include <byteswap.h>
 #include <crc32.h>
-#include "rbtree.h"
+#include <inttypes.h>
 
+#include "rbtree.h"
 #include "common.h"
 
 /* Do not use the weird XPG version of basename */
@@ -1232,8 +1233,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 		} else switch (e->sb.st_mode & S_IFMT) {
 			case S_IFDIR:
 				if (verbose) {
-					printf("\td %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\td %04o %9" PRIu64 "             %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
 							(int) (e->sb.st_uid), (int) (e->sb.st_gid),
 							e->name);
 				}
@@ -1242,8 +1243,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				break;
 			case S_IFSOCK:
 				if (verbose) {
-					printf("\ts %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\ts %04o %9" PRIu64 "             %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				write_pipe(e);
@@ -1251,8 +1252,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				break;
 			case S_IFIFO:
 				if (verbose) {
-					printf("\tp %04o %9lu             %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\tp %04o %9" PRIu64 "             %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				write_pipe(e);
@@ -1280,8 +1281,8 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				break;
 			case S_IFLNK:
 				if (verbose) {
-					printf("\tl %04o %9lu             %5d:%-3d %s -> %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
+					printf("\tl %04o %9" PRIu64 "             %5d:%-3d %s -> %s\n",
+							e->sb.st_mode & ~S_IFMT, e->sb.st_size,
 							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name,
 							e->link);
 				}
@@ -1292,9 +1293,9 @@ static void recursive_populate_directory(struct filesystem_entry *dir)
 				wrote = write_regular_file(e);
 				write_xattr_entry(e);
 				if (verbose) {
-					printf("\tf %04o %9lu (%9u) %5d:%-3d %s\n",
-							e->sb.st_mode & ~S_IFMT, (unsigned long) e->sb.st_size,
-							wrote, (int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
+					printf("\tf %04o %9" PRIu64 " (%9u) %5d:%-3d %s\n",
+							e->sb.st_mode & ~S_IFMT, e->sb.st_size, wrote,
+							(int) e->sb.st_uid, (int) e->sb.st_gid, e->name);
 				}
 				break;
 			default:
-- 
1.6.3.3

[PATCHv4 2/3] libmtd: use PRIu64 classifier for uint64_t printf arguments

[Andy Shevchenko]

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 lib/libmtd.c |   27 +++++++++++----------------
 1 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/lib/libmtd.c b/lib/libmtd.c
index 418ba29..7fabd80 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -31,9 +31,11 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/ioctl.h>
-#include <mtd/mtd-user.h>
+#include <inttypes.h>
 
+#include <mtd/mtd-user.h>
 #include <libmtd.h>
+
 #include "libmtd_int.h"
 #include "common.h"
 
@@ -1096,19 +1098,16 @@ int do_oob_op(libmtd_t desc, const struct mtd_dev_info *mtd, int fd,
 
 	max_offs = (unsigned long long)mtd->eb_cnt * mtd->eb_size;
 	if (start >= max_offs) {
-		errmsg("bad page address %llu, mtd%d has %d eraseblocks "
-		       "(%llu bytes)", (unsigned long long) start, mtd->mtd_num,
-		       mtd->eb_cnt, max_offs);
+		errmsg("bad page address %" PRIu64 ", mtd%d has %d eraseblocks (%llu bytes)",
+		       start, mtd->mtd_num, mtd->eb_cnt, max_offs);
 		errno = EINVAL;
 		return -1;
 	}
 
 	oob_offs = start & (mtd->min_io_size - 1);
 	if (oob_offs + length > mtd->oob_size || length == 0) {
-		errmsg("Cannot write %llu OOB bytes to address %llu "
-		       "(OOB offset %u) - mtd%d OOB size is only %d bytes",
-		       (unsigned long long)length, (unsigned long long)start,
-		       oob_offs, mtd->mtd_num,  mtd->oob_size);
+		errmsg("Cannot write %" PRIu64 " OOB bytes to address %" PRIu64 " (OOB offset %u) - mtd%d OOB size is only %d bytes",
+		       length, start, oob_offs, mtd->mtd_num,  mtd->oob_size);
 		errno = EINVAL;
 		return -1;
 	}
@@ -1125,10 +1124,8 @@ int do_oob_op(libmtd_t desc, const struct mtd_dev_info *mtd, int fd,
 
 		if (errno != ENOTTY ||
 		    lib->offs64_ioctls != OFFS64_IOCTLS_UNKNOWN) {
-			sys_errmsg("%s ioctl failed for mtd%d, offset %llu "
-				   "(eraseblock %llu)", cmd64_str, mtd->mtd_num,
-				   (unsigned long long)start,
-				   (unsigned long long)start / mtd->eb_size);
+			sys_errmsg("%s ioctl failed for mtd%d, offset %" PRIu64 " (eraseblock %" PRIu64 ")",
+				   cmd64_str, mtd->mtd_num, start, start / mtd->eb_size);
 		}
 
 		/*
@@ -1152,10 +1149,8 @@ int do_oob_op(libmtd_t desc, const struct mtd_dev_info *mtd, int fd,
 
 	ret = ioctl(fd, cmd, &oob);
 	if (ret < 0)
-		sys_errmsg("%s ioctl failed for mtd%d, offset %llu "
-			   "(eraseblock %llu)", cmd_str, mtd->mtd_num,
-			   (unsigned long long)start,
-			   (unsigned long long)start / mtd->eb_size);
+		sys_errmsg("%s ioctl failed for mtd%d, offset %" PRIu64 " (eraseblock %" PRIu64 ")",
+			   cmd_str, mtd->mtd_num, start, start / mtd->eb_size);
 	return ret;
 }
 
-- 
1.6.3.3

[PATCHv4 3/3] serve_image: adjust classifier and type for printf

[Andy Shevchenko]

The argument type of printf is __off64_t, meanwhile the classifier is "%ld".
We agreed to use PRIu64 in such case.

Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
---
 serve_image.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/serve_image.c b/serve_image.c
index f8f28a1..d1037a9 100644
--- a/serve_image.c
+++ b/serve_image.c
@@ -2,7 +2,6 @@
 #define _POSIX_C_SOURCE 199309
 
 #include <time.h>
-
 #include <errno.h>  	
 #include <error.h> 	
 #include <netdb.h> 	
@@ -18,6 +17,8 @@
 #include <netinet/in.h>
 #include <sys/time.h>
 #include <crc32.h>
+#include <inttypes.h>
+
 #include "mcast_image.h"
 
 int tx_rate = 80000;
@@ -126,8 +127,8 @@ int main(int argc, char **argv)
 	}
 
 	if (st.st_size % erasesize) {
-		fprintf(stderr, "Image size %ld bytes is not a multiple of erasesize %d bytes\n",
-			st.st_size, erasesize);
+		fprintf(stderr, "Image size %" PRIu64 " bytes is not a multiple of erasesize %d bytes\n"
+				st.st_size, erasesize);
 		exit(1);
 	}
 	image = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, rfd, 0);
-- 
1.6.3.3

Re: [PATCHv4 1/3] mkfs.jffs2: fix casting of __off64_t

[Mike Frysinger]

thanks for suffering me :).  all of these look fine to me:
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike

Re: [PATCHv4 1/3] mkfs.jffs2: fix casting of __off64_t

[Artem Bityutskiy]

On Wed, 2011-04-20 at 12:35 +0300, Andy Shevchenko wrote:
> The casting of __off64_t to unsigned long potentially wrong for values higher
> than ULONG_MAX.  Let's fix that by using PRIu64 classifier.
> 
> Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
> ---
>  mkfs.jffs2.c |   25 +++++++++++++------------
>  1 files changed, 13 insertions(+), 12 deletions(-)

Pushed, thank you!

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

[PATCH] serve_image: missing comma

[Brian Norris]

Somebody didn't compile-test commit 9f4e6840.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 serve_image.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/serve_image.c b/serve_image.c
index d1037a9..f7f6315 100644
--- a/serve_image.c
+++ b/serve_image.c
@@ -127,7 +127,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 %" PRIu64 " bytes is not a multiple of erasesize %d bytes\n",
 				st.st_size, erasesize);
 		exit(1);
 	}
-- 
1.7.0.4

Re: [PATCH] serve_image: missing comma

[Artem Bityutskiy]

On Thu, 2011-04-21 at 10:09 -0700, Brian Norris wrote:
> Somebody didn't compile-test commit 9f4e6840.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>

Pushed, thank you!

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCHv3 1/3] mkfs.jffs2: fix casting of __off64_t

[Mike Frysinger]

On Tue, Apr 19, 2011 at 04:34, Andy Shevchenko wrote:
> -                                       printf("\ts %04o %9lu             %5d:%-3d %s\n",
> +                                       printf("\ts %04o %" PRIu64 "             %5d:%-3d %s\n",

is there a reason for dropping the field width ?  printf("%9"PRIu64"
.....") should work fine ...
-mike

Re: [PATCHv3 1/3] mkfs.jffs2: fix casting of __off64_t

[Andy Shevchenko]

On Tue, 2011-04-19 at 10:26 -0400, ext Mike Frysinger wrote:
> On Tue, Apr 19, 2011 at 04:34, Andy Shevchenko wrote:
> > -                                       printf("\ts %04o %9lu             %5d:%-3d %s\n",
> > +                                       printf("\ts %04o %" PRIu64 "             %5d:%-3d %s\n",
> 
> is there a reason for dropping the field width ?  printf("%9"PRIu64"
> .....") should work fine ...
> -mike
If you guarantee the value not higher than 10^10, why should we use
PRIu64 at all? It's enough to have cast to unsigned long in such case.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCHv3 1/3] mkfs.jffs2: fix casting of __off64_t

[Mike Frysinger]

On Tue, Apr 19, 2011 at 10:29, Andy Shevchenko wrote:
> On Tue, 2011-04-19 at 10:26 -0400, ext Mike Frysinger wrote:
>> On Tue, Apr 19, 2011 at 04:34, Andy Shevchenko wrote:
>> > -                                       printf("\ts %04o %9lu             %5d:%-3d %s\n",
>> > +                                       printf("\ts %04o %" PRIu64 "             %5d:%-3d %s\n",
>>
>> is there a reason for dropping the field width ?  printf("%9"PRIu64"
>> .....") should work fine ...
>
> If you guarantee the value not higher than 10^10, why should we use
> PRIu64 at all? It's enough to have cast to unsigned long in such case.

it's a *minimum* field width, not a maximum.  it makes sure the output
is aligned properly for most people.
-mike

Re: [PATCHv3 1/3] mkfs.jffs2: fix casting of __off64_t

[Andy Shevchenko]

On Tue, 2011-04-19 at 10:38 -0400, ext Mike Frysinger wrote:
> On Tue, Apr 19, 2011 at 10:29, Andy Shevchenko wrote:
> > On Tue, 2011-04-19 at 10:26 -0400, ext Mike Frysinger wrote:
> >> On Tue, Apr 19, 2011 at 04:34, Andy Shevchenko wrote:
> >> > -                                       printf("\ts %04o %9lu             %5d:%-3d %s\n",
> >> > +                                       printf("\ts %04o %" PRIu64 "             %5d:%-3d %s\n",
> >>
> >> is there a reason for dropping the field width ?  printf("%9"PRIu64"
> >> .....") should work fine ...
> >
> > If you guarantee the value not higher than 10^10, why should we use
> > PRIu64 at all? It's enough to have cast to unsigned long in such case.
> 
> it's a *minimum* field width, not a maximum.  it makes sure the output
> is aligned properly for most people.
Ok, probably it's better to keep.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCHv2 1/2] serve_image: adjust classificator and type for printf

[Mike Frysinger]

On Mon, Apr 18, 2011 at 04:31, Andy Shevchenko wrote:
> The argument type of printf is __off64_t, meanwhile the classificator is "%ld".

same response here ... please use PRIu64 instead of casting to long long.
-mike