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

login
register
mail settings
Submitter Shevchenko Andriy (EXT-Teleca/Helsinki)
Date April 19, 2011, 8:34 a.m.
Message ID <971bada1c870aa0baf6e8d07784f95e31bb98213.1303202005.git.ext-andriy.shevchenko@nokia.com>
Download mbox | patch
Permalink /patch/91934/
State New
Headers show

Comments

Shevchenko Andriy (EXT-Teleca/Helsinki) - April 19, 2011, 8:34 a.m.
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(-)
Mike Frysinger - April 19, 2011, 2:26 p.m.
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
Shevchenko Andriy (EXT-Teleca/Helsinki) - April 19, 2011, 2:29 p.m.
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.
Mike Frysinger - April 19, 2011, 2:38 p.m.
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
Shevchenko Andriy (EXT-Teleca/Helsinki) - April 19, 2011, 2:42 p.m.
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.

Patch

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: