* [PATCH 1/2] mkfs: unify numeric types of main variables in main()
@ 2017-04-19 15:30 Jan Tulak
2017-04-19 15:30 ` [PATCH 2/2] mkfs: remove long long type casts Jan Tulak
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Jan Tulak @ 2017-04-19 15:30 UTC (permalink / raw)
To: linux-xfs; +Cc: Jan Tulak
Followup of my "[xfsprogs] Do we need so many data types for user input?" email.
This version has bool for flags and uses PRIu64 for printing 64bit values.
Other issues from RFC, that I didn't get a satisfactory feedback to:
* __uint64_t is used because it is declared in xfsprogs, so unless there is a
reason to not use it (e.g. the declared type is just for some special use,
or is obsolete), I'm sticking to it.
* For variables that has more than 0/1 valid values, I'm still using uint64.
Mostly because I work on patches that changes all access to these variables
to get/set functions, wrapping the opts table. Possibly with validity checks
implemented for every set attempt, and then we will have to work with uint64
anyway.
Based against 07a3e79 for-next
https://github.com/jtulak/xfsprogs-dev/tree/uint
--
In the past, when mkfs was first written, it used atoi and
similar calls, so the variables were ints. However, the situation moved
since then and in course of the time, mkfs began to use other types too.
Clean and unify it. We don't need negative values anywhere in the code and
some numbers has to be 64bit. Thus, uint64 is the best candidate as the target
type for numeric values. For flag values let's use boolean.
This patch changes variables declared at the beginning of main() +
block/sectorsize, making only minimal changes. The following patch
cleans some now-unnecessary type casts.
It would be nice to change types in some of the structures too, but
this might lead to changes outside of mkfs, so I'm skipping them for
this moment to keep it simple.
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
CHANGES:
* Flags as bool
* %lu as PRIu64
---
mkfs/xfs_mkfs.c | 200 ++++++++++++++++++++++++++++----------------------------
1 file changed, 100 insertions(+), 100 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 7a5c49f..1eb77fd 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -39,8 +39,8 @@ static int ispow2(unsigned int i);
* The configured block and sector sizes are defined as global variables so
* that they don't need to be passed to functions that require them.
*/
-unsigned int blocksize;
-unsigned int sectorsize;
+__uint64_t blocksize;
+__uint64_t sectorsize;
#define MAX_SUBOPTS 16
#define SUBOPT_NEEDS_VAL (-1LL)
@@ -758,9 +758,9 @@ calc_stripe_factors(
int dsectsz,
int lsu,
int lsectsz,
- int *dsunit,
- int *dswidth,
- int *lsunit)
+ __uint64_t *dsunit,
+ __uint64_t *dswidth,
+ __uint64_t *lsunit)
{
/* Handle data sunit/swidth options */
if ((*dsunit && !*dswidth) || (!*dsunit && *dswidth)) {
@@ -785,32 +785,32 @@ calc_stripe_factors(
usage();
}
- *dsunit = (int)BTOBBT(dsu);
+ *dsunit = BTOBBT(dsu);
*dswidth = *dsunit * dsw;
}
if (*dsunit && (*dswidth % *dsunit != 0)) {
fprintf(stderr,
- _("data stripe width (%d) must be a multiple of the "
- "data stripe unit (%d)\n"), *dswidth, *dsunit);
+ _("data stripe width (%"PRIu64") must be a multiple of the "
+ "data stripe unit (%"PRIu64")\n"), *dswidth, *dsunit);
usage();
}
/* Handle log sunit options */
if (lsu)
- *lsunit = (int)BTOBBT(lsu);
+ *lsunit = BTOBBT(lsu);
/* verify if lsu/lsunit is a multiple block size */
if (lsu % blocksize != 0) {
fprintf(stderr,
-_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
+_("log stripe unit (%d) must be a multiple of the block size (%"PRIu64")\n"),
lsu, blocksize);
exit(1);
}
if ((BBTOB(*lsunit) % blocksize != 0)) {
fprintf(stderr,
-_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
+_("log stripe unit (%"PRIu64") must be a multiple of the block size (%"PRIu64")\n"),
BBTOB(*lsunit), blocksize);
exit(1);
}
@@ -933,7 +933,7 @@ fixup_internal_log_stripe(
int sunit,
xfs_rfsblock_t *logblocks,
int blocklog,
- int *lalign)
+ __uint64_t *lalign)
{
if ((logstart % sunit) != 0) {
logstart = ((logstart + (sunit - 1))/sunit) * sunit;
@@ -1421,70 +1421,70 @@ main(
__uint64_t agsize;
xfs_alloc_rec_t *arec;
struct xfs_btree_block *block;
- int blflag;
- int blocklog;
- int bsflag;
- int bsize;
+ bool blflag;
+ __uint64_t blocklog;
+ bool bsflag;
+ __uint64_t bsize;
xfs_buf_t *buf;
- int c;
- int daflag;
- int dasize;
+ __uint64_t c;
+ bool daflag;
+ __uint64_t dasize;
xfs_rfsblock_t dblocks;
char *dfile;
- int dirblocklog;
- int dirblocksize;
+ __uint64_t dirblocklog;
+ __uint64_t dirblocksize;
char *dsize;
- int dsu;
- int dsw;
- int dsunit;
- int dswidth;
- int force_overwrite;
+ __uint64_t dsu;
+ __uint64_t dsw;
+ __uint64_t dsunit;
+ __uint64_t dswidth;
+ bool force_overwrite;
struct fsxattr fsx;
- int ilflag;
- int imaxpct;
- int imflag;
- int inodelog;
- int inopblock;
- int ipflag;
- int isflag;
- int isize;
+ bool ilflag;
+ __uint64_t imaxpct;
+ bool imflag;
+ __uint64_t inodelog;
+ __uint64_t inopblock;
+ bool ipflag;
+ bool isflag;
+ __uint64_t isize;
char *label = NULL;
- int laflag;
- int lalign;
- int ldflag;
- int liflag;
+ bool laflag;
+ __uint64_t lalign;
+ bool ldflag;
+ bool liflag;
xfs_agnumber_t logagno;
xfs_rfsblock_t logblocks;
char *logfile;
- int loginternal;
+ __uint64_t loginternal;
char *logsize;
xfs_fsblock_t logstart;
- int lvflag;
- int lsflag;
- int lsuflag;
- int lsunitflag;
- int lsectorlog;
- int lsectorsize;
- int lslflag;
- int lssflag;
- int lsu;
- int lsunit;
- int min_logblocks;
+ bool lvflag;
+ bool lsflag;
+ bool lsuflag;
+ bool lsunitflag;
+ __uint64_t lsectorlog;
+ __uint64_t lsectorsize;
+ bool lslflag;
+ bool lssflag;
+ __uint64_t lsu;
+ __uint64_t lsunit;
+ __uint64_t min_logblocks;
xfs_mount_t *mp;
xfs_mount_t mbuf;
xfs_extlen_t nbmblocks;
- int nlflag;
- int nodsflag;
- int norsflag;
+ bool nlflag;
+ bool nodsflag;
+ bool norsflag;
xfs_alloc_rec_t *nrec;
- int nsflag;
- int nvflag;
- int Nflag;
- int discard = 1;
+ bool nsflag;
+ bool nvflag;
+ bool Nflag;
+ __uint64_t discard = 1;
char *p;
char *protofile;
char *protostring;
- int qflag;
+ bool qflag;
xfs_rfsblock_t rtblocks;
xfs_extlen_t rtextblocks;
xfs_rtblock_t rtextents;
@@ -1492,13 +1492,13 @@ main(
char *rtfile;
char *rtsize;
xfs_sb_t *sbp;
- int sectorlog;
+ __uint64_t sectorlog;
__uint64_t sector_mask;
- int slflag;
- int ssflag;
+ bool slflag;
+ bool ssflag;
__uint64_t tmp_agsize;
uuid_t uuid;
- int worst_freelist;
+ __uint64_t worst_freelist;
libxfs_init_t xi;
struct fs_topology ft;
struct sb_feat_args sb_feat = {
@@ -1528,20 +1528,20 @@ main(
blocklog = blocksize = 0;
sectorlog = lsectorlog = 0;
sectorsize = lsectorsize = 0;
- agsize = daflag = dasize = dblocks = 0;
- ilflag = imflag = ipflag = isflag = 0;
- liflag = laflag = lsflag = lsuflag = lsunitflag = ldflag = lvflag = 0;
+ agsize = dasize = dblocks = 0;
+ daflag = ilflag = imflag = ipflag = isflag = false;
+ liflag = laflag = lsflag = lsuflag = lsunitflag = ldflag = lvflag = false;
loginternal = 1;
logagno = logblocks = rtblocks = rtextblocks = 0;
- Nflag = nlflag = nsflag = nvflag = 0;
+ Nflag = nlflag = nsflag = nvflag = false;
dirblocklog = dirblocksize = 0;
- qflag = 0;
+ qflag = false;
imaxpct = inodelog = inopblock = isize = 0;
dfile = logfile = rtfile = NULL;
dsize = logsize = rtsize = rtextsize = protofile = NULL;
dsu = dsw = dsunit = dswidth = lalign = lsu = lsunit = 0;
- nodsflag = norsflag = 0;
- force_overwrite = 0;
+ nodsflag = norsflag = false;
+ force_overwrite = false;
worst_freelist = 0;
memset(&fsx, 0, sizeof(fsx));
@@ -1553,7 +1553,7 @@ main(
switch (c) {
case 'C':
case 'f':
- force_overwrite = 1;
+ force_overwrite = true;
break;
case 'b':
p = optarg;
@@ -1962,7 +1962,7 @@ main(
blocksize = 1 << XFS_DFL_BLOCKSIZE_LOG;
}
if (blocksize < XFS_MIN_BLOCKSIZE || blocksize > XFS_MAX_BLOCKSIZE) {
- fprintf(stderr, _("illegal block size %d\n"), blocksize);
+ fprintf(stderr, _("illegal block size %"PRIu64"\n"), blocksize);
usage();
}
if (sb_feat.crcs_enabled && blocksize < XFS_MIN_CRC_BLOCKSIZE) {
@@ -2026,7 +2026,7 @@ _("Minimum block size for CRC enabled filesystems is %d bytes.\n"),
if ((blocksize < sectorsize) && (blocksize >= ft.lsectorsize)) {
fprintf(stderr,
-_("specified blocksize %d is less than device physical sector size %d\n"),
+_("specified blocksize %"PRIu64" is less than device physical sector size %d\n"),
blocksize, ft.psectorsize);
fprintf(stderr,
_("switching to logical sector size %d\n"),
@@ -2047,21 +2047,21 @@ _("switching to logical sector size %d\n"),
if (sectorsize < XFS_MIN_SECTORSIZE ||
sectorsize > XFS_MAX_SECTORSIZE || sectorsize > blocksize) {
if (ssflag)
- fprintf(stderr, _("illegal sector size %d\n"), sectorsize);
+ fprintf(stderr, _("illegal sector size %"PRIu64"\n"), sectorsize);
else
fprintf(stderr,
-_("block size %d cannot be smaller than logical sector size %d\n"),
+_("block size %"PRIu64" cannot be smaller than logical sector size %d\n"),
blocksize, ft.lsectorsize);
usage();
}
if (sectorsize < ft.lsectorsize) {
- fprintf(stderr, _("illegal sector size %d; hw sector is %d\n"),
+ fprintf(stderr, _("illegal sector size %"PRIu64"; hw sector is %d\n"),
sectorsize, ft.lsectorsize);
usage();
}
if (lsectorsize < XFS_MIN_SECTORSIZE ||
lsectorsize > XFS_MAX_SECTORSIZE || lsectorsize > blocksize) {
- fprintf(stderr, _("illegal log sector size %d\n"), lsectorsize);
+ fprintf(stderr, _("illegal log sector size %"PRIu64"\n"), lsectorsize);
usage();
} else if (lsectorsize > XFS_MIN_SECTORSIZE && !lsu && !lsunit) {
lsu = blocksize;
@@ -2167,7 +2167,7 @@ _("rmapbt not supported with realtime devices\n"));
if (nsflag || nlflag) {
if (dirblocksize < blocksize ||
dirblocksize > XFS_MAX_BLOCKSIZE) {
- fprintf(stderr, _("illegal directory block size %d\n"),
+ fprintf(stderr, _("illegal directory block size %"PRIu64"\n"),
dirblocksize);
usage();
}
@@ -2193,7 +2193,7 @@ _("rmapbt not supported with realtime devices\n"));
dblocks = (xfs_rfsblock_t)(dbytes >> blocklog);
if (dbytes % blocksize)
fprintf(stderr, _("warning: "
- "data length %lld not a multiple of %d, truncated to %lld\n"),
+ "data length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
(long long)dbytes, blocksize,
(long long)(dblocks << blocklog));
}
@@ -2225,7 +2225,7 @@ _("rmapbt not supported with realtime devices\n"));
logblocks = (xfs_rfsblock_t)(logbytes >> blocklog);
if (logbytes % blocksize)
fprintf(stderr,
- _("warning: log length %lld not a multiple of %d, truncated to %lld\n"),
+ _("warning: log length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
(long long)logbytes, blocksize,
(long long)(logblocks << blocklog));
}
@@ -2242,7 +2242,7 @@ _("rmapbt not supported with realtime devices\n"));
rtblocks = (xfs_rfsblock_t)(rtbytes >> blocklog);
if (rtbytes % blocksize)
fprintf(stderr,
- _("warning: rt length %lld not a multiple of %d, truncated to %lld\n"),
+ _("warning: rt length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
(long long)rtbytes, blocksize,
(long long)(rtblocks << blocklog));
}
@@ -2255,7 +2255,7 @@ _("rmapbt not supported with realtime devices\n"));
rtextbytes = getnum(rtextsize, &ropts, R_EXTSIZE);
if (rtextbytes % blocksize) {
fprintf(stderr,
- _("illegal rt extent size %lld, not a multiple of %d\n"),
+ _("illegal rt extent size %lld, not a multiple of %"PRIu64"\n"),
(long long)rtextbytes, blocksize);
usage();
}
@@ -2298,16 +2298,16 @@ _("rmapbt not supported with realtime devices\n"));
isize > XFS_DINODE_MAX_SIZE) {
int maxsz;
- fprintf(stderr, _("illegal inode size %d\n"), isize);
+ fprintf(stderr, _("illegal inode size %"PRIu64"\n"), isize);
maxsz = MIN(blocksize / XFS_MIN_INODE_PERBLOCK,
XFS_DINODE_MAX_SIZE);
if (XFS_DINODE_MIN_SIZE == maxsz)
fprintf(stderr,
- _("allowable inode size with %d byte blocks is %d\n"),
+ _("allowable inode size with %"PRIu64" byte blocks is %d\n"),
blocksize, XFS_DINODE_MIN_SIZE);
else
fprintf(stderr,
- _("allowable inode size with %d byte blocks is between %d and %d\n"),
+ _("allowable inode size with %"PRIu64" byte blocks is between %d and %d\n"),
blocksize, XFS_DINODE_MIN_SIZE, maxsz);
exit(1);
}
@@ -2411,19 +2411,19 @@ _("rmapbt not supported with realtime devices\n"));
if (xi.dbsize > sectorsize) {
fprintf(stderr, _(
-"Warning: the data subvolume sector size %u is less than the sector size \n\
+"Warning: the data subvolume sector size %"PRIu64" is less than the sector size \n\
reported by the device (%u).\n"),
sectorsize, xi.dbsize);
}
if (!loginternal && xi.lbsize > lsectorsize) {
fprintf(stderr, _(
-"Warning: the log subvolume sector size %u is less than the sector size\n\
+"Warning: the log subvolume sector size %"PRIu64" is less than the sector size\n\
reported by the device (%u).\n"),
lsectorsize, xi.lbsize);
}
if (rtsize && xi.rtsize > 0 && xi.rtbsize > sectorsize) {
fprintf(stderr, _(
-"Warning: the realtime subvolume sector size %u is less than the sector size\n\
+"Warning: the realtime subvolume sector size %"PRIu64" is less than the sector size\n\
reported by the device (%u).\n"),
sectorsize, xi.rtbsize);
}
@@ -2453,14 +2453,14 @@ reported by the device (%u).\n"),
if (dsunit) {
if (ft.dsunit && ft.dsunit != dsunit) {
fprintf(stderr,
- _("%s: Specified data stripe unit %d "
+ _("%s: Specified data stripe unit %"PRIu64" "
"is not the same as the volume stripe "
"unit %d\n"),
progname, dsunit, ft.dsunit);
}
if (ft.dswidth && ft.dswidth != dswidth) {
fprintf(stderr,
- _("%s: Specified data stripe width %d "
+ _("%s: Specified data stripe width %"PRIu64" "
"is not the same as the volume stripe "
"width %d\n"),
progname, dswidth, ft.dswidth);
@@ -2478,7 +2478,7 @@ reported by the device (%u).\n"),
*/
if (agsize % blocksize) {
fprintf(stderr,
- _("agsize (%lld) not a multiple of fs blk size (%d)\n"),
+ _("agsize (%lld) not a multiple of fs blk size (%"PRIu64")\n"),
(long long)agsize, blocksize);
usage();
}
@@ -2528,7 +2528,7 @@ reported by the device (%u).\n"),
(dblocks % agsize != 0);
if (dasize)
fprintf(stderr,
- _("agsize rounded to %lld, swidth = %d\n"),
+ _("agsize rounded to %lld, swidth = %"PRIu64"\n"),
(long long)agsize, dswidth);
} else {
if (nodsflag) {
@@ -2585,8 +2585,8 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
dsunit = dswidth = 0;
else {
fprintf(stderr,
- _("%s: Stripe unit(%d) or stripe width(%d) is "
- "not a multiple of the block size(%d)\n"),
+ _("%s: Stripe unit(%"PRIu64") or stripe width(%"PRIu64") is "
+ "not a multiple of the block size(%"PRIu64")\n"),
progname, BBTOB(dsunit), BBTOB(dswidth),
blocksize);
exit(1);
@@ -2626,7 +2626,7 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
/* Warn only if specified on commandline */
if (lsuflag || lsunitflag) {
fprintf(stderr,
- _("log stripe unit (%d bytes) is too large (maximum is 256KiB)\n"),
+ _("log stripe unit (%"PRIu64" bytes) is too large (maximum is 256KiB)\n"),
(lsunit * blocksize));
fprintf(stderr,
_("log stripe unit adjusted to 32KiB\n"));
@@ -2771,14 +2771,14 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
if (!qflag || Nflag) {
printf(_(
- "meta-data=%-22s isize=%-6d agcount=%lld, agsize=%lld blks\n"
- " =%-22s sectsz=%-5u attr=%u, projid32bit=%u\n"
+ "meta-data=%-22s isize=%-6lu agcount=%lld, agsize=%lld blks\n"
+ " =%-22s sectsz=%-5lu attr=%u, projid32bit=%u\n"
" =%-22s crc=%-8u finobt=%u, sparse=%u, rmapbt=%u, reflink=%u\n"
- "data =%-22s bsize=%-6u blocks=%llu, imaxpct=%u\n"
- " =%-22s sunit=%-6u swidth=%u blks\n"
- "naming =version %-14u bsize=%-6u ascii-ci=%d ftype=%d\n"
+ "data =%-22s bsize=%-6lu blocks=%llu, imaxpct=%"PRIu64"\n"
+ " =%-22s sunit=%-6lu swidth=%"PRIu64" blks\n"
+ "naming =version %-14u bsize=%-6lu ascii-ci=%d ftype=%d\n"
"log =%-22s bsize=%-6d blocks=%lld, version=%d\n"
- " =%-22s sectsz=%-5u sunit=%d blks, lazy-count=%d\n"
+ " =%-22s sectsz=%-5lu sunit=%"PRIu64" blks, lazy-count=%d\n"
"realtime =%-22s extsz=%-6d blocks=%lld, rtextents=%lld\n"),
dfile, isize, (long long)agcount, (long long)agsize,
"", sectorsize, sb_feat.attr_version,
--
2.1.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] mkfs: remove long long type casts
2017-04-19 15:30 [PATCH 1/2] mkfs: unify numeric types of main variables in main() Jan Tulak
@ 2017-04-19 15:30 ` Jan Tulak
2017-04-20 8:33 ` [PATCH 2/2 v2] " Jan Tulak
2017-04-25 1:40 ` [PATCH 2/2] " Luis R. Rodriguez
2017-04-20 0:09 ` [PATCH 1/2] mkfs: unify numeric types of main variables in main() Dave Chinner
` (2 subsequent siblings)
3 siblings, 2 replies; 17+ messages in thread
From: Jan Tulak @ 2017-04-19 15:30 UTC (permalink / raw)
To: linux-xfs; +Cc: Jan Tulak
We have uint64, why cast it to long long for prints? Just print it as
it is.
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
EDIT:
* convert %lu to PRIu64
---
mkfs/xfs_mkfs.c | 144 ++++++++++++++++++++++++++++----------------------------
1 file changed, 72 insertions(+), 72 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 1eb77fd..994bd68 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -916,9 +916,9 @@ fixup_log_stripe_unit(
}
*logblocks = tmp_logblocks;
} else {
- fprintf(stderr, _("log size %lld is not a multiple "
+ fprintf(stderr, _("log size %"PRIu64" is not a multiple "
"of the log stripe unit %d\n"),
- (long long) *logblocks, sunit);
+ *logblocks, sunit);
usage();
}
}
@@ -945,7 +945,7 @@ fixup_internal_log_stripe(
if (*logblocks > agsize - XFS_FSB_TO_AGBNO(mp, logstart)) {
fprintf(stderr,
_("Due to stripe alignment, the internal log size "
- "(%lld) is too large.\n"), (long long) *logblocks);
+ "(%"PRIu64") is too large.\n"), *logblocks);
fprintf(stderr, _("Must fit within an allocation group.\n"));
usage();
}
@@ -957,20 +957,20 @@ validate_log_size(__uint64_t logblocks, int blocklog, int min_logblocks)
{
if (logblocks < min_logblocks) {
fprintf(stderr,
- _("log size %lld blocks too small, minimum size is %d blocks\n"),
- (long long)logblocks, min_logblocks);
+ _("log size %"PRIu64" blocks too small, minimum size is %d blocks\n"),
+ logblocks, min_logblocks);
usage();
}
if (logblocks > XFS_MAX_LOG_BLOCKS) {
fprintf(stderr,
- _("log size %lld blocks too large, maximum size is %lld blocks\n"),
- (long long)logblocks, XFS_MAX_LOG_BLOCKS);
+ _("log size %"PRIu64" blocks too large, maximum size is %lld blocks\n"),
+ logblocks, XFS_MAX_LOG_BLOCKS);
usage();
}
if ((logblocks << blocklog) > XFS_MAX_LOG_BYTES) {
fprintf(stderr,
- _("log size %lld bytes too large, maximum size is %lld bytes\n"),
- (long long)(logblocks << blocklog), XFS_MAX_LOG_BYTES);
+ _("log size %"PRIu64" bytes too large, maximum size is %lld bytes\n"),
+ (logblocks << blocklog), XFS_MAX_LOG_BYTES);
usage();
}
}
@@ -1006,43 +1006,43 @@ validate_ag_geometry(
{
if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
fprintf(stderr,
- _("agsize (%lld blocks) too small, need at least %lld blocks\n"),
- (long long)agsize,
- (long long)XFS_AG_MIN_BLOCKS(blocklog));
+ _("agsize (%"PRIu64" blocks) too small, need at least %"PRIu64" blocks\n"),
+ agsize,
+ (__uint64_t)XFS_AG_MIN_BLOCKS(blocklog));
usage();
}
if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
fprintf(stderr,
- _("agsize (%lld blocks) too big, maximum is %lld blocks\n"),
- (long long)agsize,
- (long long)XFS_AG_MAX_BLOCKS(blocklog));
+ _("agsize (%"PRIu64" blocks) too big, maximum is %"PRIu64" blocks\n"),
+ agsize,
+ (__uint64_t)XFS_AG_MAX_BLOCKS(blocklog));
usage();
}
if (agsize > dblocks) {
fprintf(stderr,
- _("agsize (%lld blocks) too big, data area is %lld blocks\n"),
- (long long)agsize, (long long)dblocks);
+ _("agsize (%"PRIu64" blocks) too big, data area is %"PRIu64" blocks\n"),
+ agsize, dblocks);
usage();
}
if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
fprintf(stderr,
- _("too many allocation groups for size = %lld\n"),
- (long long)agsize);
- fprintf(stderr, _("need at most %lld allocation groups\n"),
- (long long)(dblocks / XFS_AG_MIN_BLOCKS(blocklog) +
+ _("too many allocation groups for size = %"PRIu64"\n"),
+ agsize);
+ fprintf(stderr, _("need at most %"PRIu64" allocation groups\n"),
+ (__uint64_t) (dblocks / XFS_AG_MIN_BLOCKS(blocklog) +
(dblocks % XFS_AG_MIN_BLOCKS(blocklog) != 0)));
usage();
}
if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
fprintf(stderr,
- _("too few allocation groups for size = %lld\n"), (long long)agsize);
+ _("too few allocation groups for size = %"PRIu64"\n"), agsize);
fprintf(stderr,
- _("need at least %lld allocation groups\n"),
- (long long)(dblocks / XFS_AG_MAX_BLOCKS(blocklog) +
+ _("need at least %"PRIu64" allocation groups\n"),
+ (__uint64_t)(dblocks / XFS_AG_MAX_BLOCKS(blocklog) +
(dblocks % XFS_AG_MAX_BLOCKS(blocklog) != 0)));
usage();
}
@@ -1054,9 +1054,9 @@ validate_ag_geometry(
if ( dblocks % agsize != 0 &&
(dblocks % agsize < XFS_AG_MIN_BLOCKS(blocklog))) {
fprintf(stderr,
- _("last AG size %lld blocks too small, minimum size is %lld blocks\n"),
- (long long)(dblocks % agsize),
- (long long)XFS_AG_MIN_BLOCKS(blocklog));
+ _("last AG size %"PRIu64" blocks too small, minimum size is %"PRIu64" blocks\n"),
+ (dblocks % agsize),
+ (__uint64_t)XFS_AG_MIN_BLOCKS(blocklog));
usage();
}
@@ -1065,8 +1065,8 @@ validate_ag_geometry(
*/
if (agcount > XFS_MAX_AGNUMBER + 1) {
fprintf(stderr,
- _("%lld allocation groups is too many, maximum is %lld\n"),
- (long long)agcount, (long long)XFS_MAX_AGNUMBER + 1);
+ _("%"PRIu64" allocation groups is too many, maximum is %"PRIu64"\n"),
+ agcount, (__uint64_t)XFS_MAX_AGNUMBER + 1);
usage();
}
}
@@ -2186,16 +2186,16 @@ _("rmapbt not supported with realtime devices\n"));
dbytes = getnum(dsize, &dopts, D_SIZE);
if (dbytes % XFS_MIN_BLOCKSIZE) {
fprintf(stderr,
- _("illegal data length %lld, not a multiple of %d\n"),
- (long long)dbytes, XFS_MIN_BLOCKSIZE);
+ _("illegal data length %"PRIu64", not a multiple of %d\n"),
+ dbytes, XFS_MIN_BLOCKSIZE);
usage();
}
dblocks = (xfs_rfsblock_t)(dbytes >> blocklog);
if (dbytes % blocksize)
fprintf(stderr, _("warning: "
- "data length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
- (long long)dbytes, blocksize,
- (long long)(dblocks << blocklog));
+ "data length %"PRIu64" not a multiple of %"PRIu64", truncated to %"PRIu64"\n"),
+ dbytes, blocksize,
+ (__uint64_t)(dblocks << blocklog));
}
if (ipflag) {
inodelog = blocklog - libxfs_highbit32(inopblock);
@@ -2218,16 +2218,16 @@ _("rmapbt not supported with realtime devices\n"));
logbytes = getnum(logsize, &lopts, L_SIZE);
if (logbytes % XFS_MIN_BLOCKSIZE) {
fprintf(stderr,
- _("illegal log length %lld, not a multiple of %d\n"),
- (long long)logbytes, XFS_MIN_BLOCKSIZE);
+ _("illegal log length %"PRIu64", not a multiple of %d\n"),
+ logbytes, XFS_MIN_BLOCKSIZE);
usage();
}
logblocks = (xfs_rfsblock_t)(logbytes >> blocklog);
if (logbytes % blocksize)
fprintf(stderr,
- _("warning: log length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
- (long long)logbytes, blocksize,
- (long long)(logblocks << blocklog));
+ _("warning: log length %"PRIu64" not a multiple of %"PRIu64", truncated to %"PRIu64"\n"),
+ logbytes, blocksize,
+ (__uint64_t)(logblocks << blocklog));
}
if (rtsize) {
__uint64_t rtbytes;
@@ -2235,16 +2235,16 @@ _("rmapbt not supported with realtime devices\n"));
rtbytes = getnum(rtsize, &ropts, R_SIZE);
if (rtbytes % XFS_MIN_BLOCKSIZE) {
fprintf(stderr,
- _("illegal rt length %lld, not a multiple of %d\n"),
- (long long)rtbytes, XFS_MIN_BLOCKSIZE);
+ _("illegal rt length %"PRIu64", not a multiple of %d\n"),
+ rtbytes, XFS_MIN_BLOCKSIZE);
usage();
}
rtblocks = (xfs_rfsblock_t)(rtbytes >> blocklog);
if (rtbytes % blocksize)
fprintf(stderr,
- _("warning: rt length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
- (long long)rtbytes, blocksize,
- (long long)(rtblocks << blocklog));
+ _("warning: rt length %"PRIu64" not a multiple of %"PRIu64", truncated to %"PRIu64"\n"),
+ rtbytes, blocksize,
+ (__uint64_t)(rtblocks << blocklog));
}
/*
* If specified, check rt extent size against its constraints.
@@ -2255,8 +2255,8 @@ _("rmapbt not supported with realtime devices\n"));
rtextbytes = getnum(rtextsize, &ropts, R_EXTSIZE);
if (rtextbytes % blocksize) {
fprintf(stderr,
- _("illegal rt extent size %lld, not a multiple of %"PRIu64"\n"),
- (long long)rtextbytes, blocksize);
+ _("illegal rt extent size %"PRIu64", not a multiple of %"PRIu64"\n"),
+ rtextbytes, blocksize);
usage();
}
rtextblocks = (xfs_extlen_t)(rtextbytes >> blocklog);
@@ -2383,8 +2383,8 @@ _("rmapbt not supported with realtime devices\n"));
if (dsize && xi.dsize > 0 && dblocks > DTOBT(xi.dsize)) {
fprintf(stderr,
_("size %s specified for data subvolume is too large, "
- "maximum is %lld blocks\n"),
- dsize, (long long)DTOBT(xi.dsize));
+ "maximum is %"PRIu64" blocks\n"),
+ dsize, (__uint64_t)DTOBT(xi.dsize));
usage();
} else if (!dsize && xi.dsize > 0)
dblocks = DTOBT(xi.dsize);
@@ -2394,8 +2394,8 @@ _("rmapbt not supported with realtime devices\n"));
}
if (dblocks < XFS_MIN_DATA_BLOCKS) {
fprintf(stderr,
- _("size %lld of data subvolume is too small, minimum %d blocks\n"),
- (long long)dblocks, XFS_MIN_DATA_BLOCKS);
+ _("size %"PRIu64" of data subvolume is too small, minimum %d blocks\n"),
+ dblocks, XFS_MIN_DATA_BLOCKS);
usage();
}
@@ -2431,8 +2431,8 @@ reported by the device (%u).\n"),
if (rtsize && xi.rtsize > 0 && rtblocks > DTOBT(xi.rtsize)) {
fprintf(stderr,
_("size %s specified for rt subvolume is too large, "
- "maximum is %lld blocks\n"),
- rtsize, (long long)DTOBT(xi.rtsize));
+ "maximum is %"PRIu64" blocks\n"),
+ rtsize, (__uint64_t)DTOBT(xi.rtsize));
usage();
} else if (!rtsize && xi.rtsize > 0)
rtblocks = DTOBT(xi.rtsize);
@@ -2478,8 +2478,8 @@ reported by the device (%u).\n"),
*/
if (agsize % blocksize) {
fprintf(stderr,
- _("agsize (%lld) not a multiple of fs blk size (%"PRIu64")\n"),
- (long long)agsize, blocksize);
+ _("agsize (%"PRIu64") not a multiple of fs blk size (%"PRIu64")\n"),
+ agsize, blocksize);
usage();
}
agsize /= blocksize;
@@ -2528,8 +2528,8 @@ reported by the device (%u).\n"),
(dblocks % agsize != 0);
if (dasize)
fprintf(stderr,
- _("agsize rounded to %lld, swidth = %"PRIu64"\n"),
- (long long)agsize, dswidth);
+ _("agsize rounded to %"PRIu64", swidth = %"PRIu64"\n"),
+ agsize, dswidth);
} else {
if (nodsflag) {
dsunit = dswidth = 0;
@@ -2645,8 +2645,8 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
min_logblocks = MAX(min_logblocks, XFS_MIN_LOG_BYTES>>blocklog);
if (logsize && xi.logBBsize > 0 && logblocks > DTOBT(xi.logBBsize)) {
fprintf(stderr,
-_("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
- logsize, (long long)DTOBT(xi.logBBsize));
+_("size %s specified for log subvolume is too large, maximum is %"PRIu64" blocks\n"),
+ logsize, (__uint64_t)DTOBT(xi.logBBsize));
usage();
} else if (!logsize && xi.logBBsize > 0) {
logblocks = DTOBT(xi.logBBsize);
@@ -2655,8 +2655,8 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
_("size specified for non-existent log subvolume\n"));
usage();
} else if (loginternal && logsize && logblocks >= dblocks) {
- fprintf(stderr, _("size %lld too large for internal log\n"),
- (long long)logblocks);
+ fprintf(stderr, _("size %"PRIu64" too large for internal log\n"),
+ logblocks);
usage();
} else if (!loginternal && !xi.logdev) {
logblocks = 0;
@@ -2733,16 +2733,16 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
}
if (logblocks > agsize - libxfs_prealloc_blocks(mp)) {
fprintf(stderr,
- _("internal log size %lld too large, must fit in allocation group\n"),
- (long long)logblocks);
+ _("internal log size %"PRIu64" too large, must fit in allocation group\n"),
+ logblocks);
usage();
}
if (laflag) {
if (logagno >= agcount) {
fprintf(stderr,
- _("log ag number %d too large, must be less than %lld\n"),
- logagno, (long long)agcount);
+ _("log ag number %d too large, must be less than %"PRIu64"\n"),
+ logagno, agcount);
usage();
}
} else
@@ -2771,29 +2771,29 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
if (!qflag || Nflag) {
printf(_(
- "meta-data=%-22s isize=%-6lu agcount=%lld, agsize=%lld blks\n"
+ "meta-data=%-22s isize=%-6lu agcount=%"PRIu64", agsize=%"PRIu64" blks\n"
" =%-22s sectsz=%-5lu attr=%u, projid32bit=%u\n"
" =%-22s crc=%-8u finobt=%u, sparse=%u, rmapbt=%u, reflink=%u\n"
- "data =%-22s bsize=%-6lu blocks=%llu, imaxpct=%"PRIu64"\n"
+ "data =%-22s bsize=%-6lu blocks=%"PRIu64", imaxpct=%"PRIu64"\n"
" =%-22s sunit=%-6lu swidth=%"PRIu64" blks\n"
"naming =version %-14u bsize=%-6lu ascii-ci=%d ftype=%d\n"
- "log =%-22s bsize=%-6d blocks=%lld, version=%d\n"
+ "log =%-22s bsize=%-6d blocks=%"PRIu64", version=%d\n"
" =%-22s sectsz=%-5lu sunit=%"PRIu64" blks, lazy-count=%d\n"
- "realtime =%-22s extsz=%-6d blocks=%lld, rtextents=%lld\n"),
- dfile, isize, (long long)agcount, (long long)agsize,
+ "realtime =%-22s extsz=%-6d blocks=%"PRIu64", rtextents=%"PRIu64"\n"),
+ dfile, isize, agcount, agsize,
"", sectorsize, sb_feat.attr_version,
!sb_feat.projid16bit,
"", sb_feat.crcs_enabled, sb_feat.finobt, sb_feat.spinodes,
sb_feat.rmapbt, sb_feat.reflink,
- "", blocksize, (long long)dblocks, imaxpct,
+ "", blocksize, dblocks, imaxpct,
"", dsunit, dswidth,
sb_feat.dir_version, dirblocksize, sb_feat.nci,
sb_feat.dirftype,
- logfile, 1 << blocklog, (long long)logblocks,
+ logfile, 1 << blocklog, logblocks,
sb_feat.log_version, "", lsectorsize, lsunit,
sb_feat.lazy_sb_counters,
rtfile, rtextblocks << blocklog,
- (long long)rtblocks, (long long)rtextents);
+ rtblocks, rtextents);
if (Nflag)
exit(0);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] mkfs: unify numeric types of main variables in main()
2017-04-19 15:30 [PATCH 1/2] mkfs: unify numeric types of main variables in main() Jan Tulak
2017-04-19 15:30 ` [PATCH 2/2] mkfs: remove long long type casts Jan Tulak
@ 2017-04-20 0:09 ` Dave Chinner
2017-04-20 8:06 ` Jan Tulak
2017-04-20 8:33 ` [PATCH 1/2 v2] " Jan Tulak
2017-04-20 13:58 ` [PATCH 1/2 v3] " Jan Tulak
3 siblings, 1 reply; 17+ messages in thread
From: Dave Chinner @ 2017-04-20 0:09 UTC (permalink / raw)
To: Jan Tulak; +Cc: linux-xfs
On Wed, Apr 19, 2017 at 05:30:24PM +0200, Jan Tulak wrote:
> Followup of my "[xfsprogs] Do we need so many data types for user input?" email.
> This version has bool for flags and uses PRIu64 for printing 64bit values.
>
> Other issues from RFC, that I didn't get a satisfactory feedback to:
> * __uint64_t is used because it is declared in xfsprogs, so unless there is a
> reason to not use it (e.g. the declared type is just for some special use,
> or is obsolete), I'm sticking to it.
uint64_t is a standard C99 type. Please use it over an internal
type that we plan to get rid of. i.e.
http://oss.sgi.com/archives/xfs/2016-01/msg00386.html
These patches were never finalised/finished because of conflicts
with all the COW work that was pending at the time. This type
conversion still needs to be picked up and finished...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] mkfs: unify numeric types of main variables in main()
2017-04-20 0:09 ` [PATCH 1/2] mkfs: unify numeric types of main variables in main() Dave Chinner
@ 2017-04-20 8:06 ` Jan Tulak
0 siblings, 0 replies; 17+ messages in thread
From: Jan Tulak @ 2017-04-20 8:06 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Thu, Apr 20, 2017 at 2:09 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Wed, Apr 19, 2017 at 05:30:24PM +0200, Jan Tulak wrote:
>> Followup of my "[xfsprogs] Do we need so many data types for user input?" email.
>> This version has bool for flags and uses PRIu64 for printing 64bit values.
>>
>> Other issues from RFC, that I didn't get a satisfactory feedback to:
>> * __uint64_t is used because it is declared in xfsprogs, so unless there is a
>> reason to not use it (e.g. the declared type is just for some special use,
>> or is obsolete), I'm sticking to it.
>
> uint64_t is a standard C99 type. Please use it over an internal
> type that we plan to get rid of. i.e.
>
> http://oss.sgi.com/archives/xfs/2016-01/msg00386.html
>
> These patches were never finalised/finished because of conflicts
> with all the COW work that was pending at the time. This type
> conversion still needs to be picked up and finished...
>
Thanks, Dave. I will turn it to the standard type.
Jan
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
--
Jan Tulak
jtulak@redhat.com / jan@tulak.me
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2 v2] mkfs: unify numeric types of main variables in main()
2017-04-19 15:30 [PATCH 1/2] mkfs: unify numeric types of main variables in main() Jan Tulak
2017-04-19 15:30 ` [PATCH 2/2] mkfs: remove long long type casts Jan Tulak
2017-04-20 0:09 ` [PATCH 1/2] mkfs: unify numeric types of main variables in main() Dave Chinner
@ 2017-04-20 8:33 ` Jan Tulak
2017-04-20 13:28 ` Luis R. Rodriguez
2017-04-26 3:58 ` Eric Sandeen
2017-04-20 13:58 ` [PATCH 1/2 v3] " Jan Tulak
3 siblings, 2 replies; 17+ messages in thread
From: Jan Tulak @ 2017-04-20 8:33 UTC (permalink / raw)
To: linux-xfs; +Cc: Jan Tulak
In the past, when mkfs was first written, it used atoi and
similar calls, so the variables were ints. However, the situation moved
since then and in course of the time, mkfs began to use other types too.
Clean and unify it. We don't need negative values anywhere in the code and
some numbers has to be 64bit. Thus, uint64_t is the best candidate as the target
type for numeric values. The existing uses of __uint64_t in mkfs change to the
standard uint64_t type. For flag values let's use boolean.
This patch changes variables declared at the beginning of main() +
block/sectorsize, making only minimal changes. The following patch
cleans some now-unnecessary type casts.
It would be nice to change types in some of the structures too, but
this might lead to changes outside of mkfs, so I'm skipping them for
this moment to keep it simple.
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
CHANGES:
* __uint64_t -> uint64_t
* Add other places, where we still used long long and I forgot them
before (cvtnum, getnum, subopt_param)
Previous changes
* Flags as bool
* %lu as PRIu64
---
include/xfs_multidisk.h | 2 +-
mkfs/xfs_mkfs.c | 266 ++++++++++++++++++++++++------------------------
2 files changed, 134 insertions(+), 134 deletions(-)
diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h
index ce9bbce..ab74536 100644
--- a/include/xfs_multidisk.h
+++ b/include/xfs_multidisk.h
@@ -57,7 +57,7 @@
#define XFS_NOMULTIDISK_AGLOG 2 /* 4 AGs */
#define XFS_MULTIDISK_AGCOUNT (1 << XFS_MULTIDISK_AGLOG)
-extern long long cvtnum(unsigned int blksize, unsigned int sectsize,
+extern __uint64_t cvtnum(unsigned int blksize, unsigned int sectsize,
const char *str);
/* proto.c */
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 7a5c49f..40a32be 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -39,8 +39,8 @@ static int ispow2(unsigned int i);
* The configured block and sector sizes are defined as global variables so
* that they don't need to be passed to functions that require them.
*/
-unsigned int blocksize;
-unsigned int sectorsize;
+uint64_t blocksize;
+uint64_t sectorsize;
#define MAX_SUBOPTS 16
#define SUBOPT_NEEDS_VAL (-1LL)
@@ -138,9 +138,9 @@ struct opt_params {
bool convert;
bool is_power_2;
int conflicts[MAX_CONFLICTS];
- long long minval;
- long long maxval;
- long long defaultval;
+ uint64_t minval;
+ uint64_t maxval;
+ uint64_t defaultval;
} subopt_params[MAX_SUBOPTS];
};
@@ -724,9 +724,9 @@ struct opt_params mopts = {
},
};
-#define TERABYTES(count, blog) ((__uint64_t)(count) << (40 - (blog)))
-#define GIGABYTES(count, blog) ((__uint64_t)(count) << (30 - (blog)))
-#define MEGABYTES(count, blog) ((__uint64_t)(count) << (20 - (blog)))
+#define TERABYTES(count, blog) ((uint64_t)(count) << (40 - (blog)))
+#define GIGABYTES(count, blog) ((uint64_t)(count) << (30 - (blog)))
+#define MEGABYTES(count, blog) ((uint64_t)(count) << (20 - (blog)))
/*
* Use this macro before we have superblock and mount structure
@@ -758,9 +758,9 @@ calc_stripe_factors(
int dsectsz,
int lsu,
int lsectsz,
- int *dsunit,
- int *dswidth,
- int *lsunit)
+ uint64_t *dsunit,
+ uint64_t *dswidth,
+ uint64_t *lsunit)
{
/* Handle data sunit/swidth options */
if ((*dsunit && !*dswidth) || (!*dsunit && *dswidth)) {
@@ -785,32 +785,32 @@ calc_stripe_factors(
usage();
}
- *dsunit = (int)BTOBBT(dsu);
+ *dsunit = BTOBBT(dsu);
*dswidth = *dsunit * dsw;
}
if (*dsunit && (*dswidth % *dsunit != 0)) {
fprintf(stderr,
- _("data stripe width (%d) must be a multiple of the "
- "data stripe unit (%d)\n"), *dswidth, *dsunit);
+ _("data stripe width (%"PRIu64") must be a multiple of the "
+ "data stripe unit (%"PRIu64")\n"), *dswidth, *dsunit);
usage();
}
/* Handle log sunit options */
if (lsu)
- *lsunit = (int)BTOBBT(lsu);
+ *lsunit = BTOBBT(lsu);
/* verify if lsu/lsunit is a multiple block size */
if (lsu % blocksize != 0) {
fprintf(stderr,
-_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
+_("log stripe unit (%d) must be a multiple of the block size (%"PRIu64")\n"),
lsu, blocksize);
exit(1);
}
if ((BBTOB(*lsunit) % blocksize != 0)) {
fprintf(stderr,
-_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
+_("log stripe unit (%"PRIu64") must be a multiple of the block size (%"PRIu64")\n"),
BBTOB(*lsunit), blocksize);
exit(1);
}
@@ -897,7 +897,7 @@ fixup_log_stripe_unit(
xfs_rfsblock_t *logblocks,
int blocklog)
{
- __uint64_t tmp_logblocks;
+ uint64_t tmp_logblocks;
/*
* Make sure that the log size is a multiple of the stripe unit
@@ -929,11 +929,11 @@ fixup_internal_log_stripe(
xfs_mount_t *mp,
int lsflag,
xfs_fsblock_t logstart,
- __uint64_t agsize,
+ uint64_t agsize,
int sunit,
xfs_rfsblock_t *logblocks,
int blocklog,
- int *lalign)
+ uint64_t *lalign)
{
if ((logstart % sunit) != 0) {
logstart = ((logstart + (sunit - 1))/sunit) * sunit;
@@ -953,7 +953,7 @@ fixup_internal_log_stripe(
}
void
-validate_log_size(__uint64_t logblocks, int blocklog, int min_logblocks)
+validate_log_size(uint64_t logblocks, int blocklog, int min_logblocks)
{
if (logblocks < min_logblocks) {
fprintf(stderr,
@@ -978,7 +978,7 @@ validate_log_size(__uint64_t logblocks, int blocklog, int min_logblocks)
static int
calc_default_imaxpct(
int blocklog,
- __uint64_t dblocks)
+ uint64_t dblocks)
{
/*
* This returns the % of the disk space that is used for
@@ -1000,9 +1000,9 @@ calc_default_imaxpct(
static void
validate_ag_geometry(
int blocklog,
- __uint64_t dblocks,
- __uint64_t agsize,
- __uint64_t agcount)
+ uint64_t dblocks,
+ uint64_t agsize,
+ uint64_t agcount)
{
if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
fprintf(stderr,
@@ -1131,8 +1131,8 @@ zero_old_xfs_structures(
i != sb.sb_blocklog)
goto done;
- if (sb.sb_dblocks > ((__uint64_t)sb.sb_agcount * sb.sb_agblocks) ||
- sb.sb_dblocks < ((__uint64_t)(sb.sb_agcount - 1) *
+ if (sb.sb_dblocks > ((uint64_t)sb.sb_agcount * sb.sb_agblocks) ||
+ sb.sb_dblocks < ((uint64_t)(sb.sb_agcount - 1) *
sb.sb_agblocks + XFS_MIN_AG_BLOCKS))
goto done;
@@ -1152,7 +1152,7 @@ done:
}
static void
-discard_blocks(dev_t dev, __uint64_t nsectors)
+discard_blocks(dev_t dev, uint64_t nsectors)
{
int fd;
@@ -1336,14 +1336,14 @@ check_opt(
}
}
-static long long
+static uint64_t
getnum(
const char *str,
struct opt_params *opts,
int index)
{
struct subopt_param *sp = &opts->subopt_params[index];
- long long c;
+ uint64_t c;
check_opt(opts, index, false);
/* empty strings might just return a default value */
@@ -1414,77 +1414,77 @@ main(
int argc,
char **argv)
{
- __uint64_t agcount;
+ uint64_t agcount;
xfs_agf_t *agf;
xfs_agi_t *agi;
xfs_agnumber_t agno;
- __uint64_t agsize;
+ uint64_t agsize;
xfs_alloc_rec_t *arec;
struct xfs_btree_block *block;
- int blflag;
- int blocklog;
- int bsflag;
- int bsize;
+ bool blflag;
+ uint64_t blocklog;
+ bool bsflag;
+ uint64_t bsize;
xfs_buf_t *buf;
- int c;
- int daflag;
- int dasize;
+ uint64_t c;
+ bool daflag;
+ uint64_t dasize;
xfs_rfsblock_t dblocks;
char *dfile;
- int dirblocklog;
- int dirblocksize;
+ uint64_t dirblocklog;
+ uint64_t dirblocksize;
char *dsize;
- int dsu;
- int dsw;
- int dsunit;
- int dswidth;
- int force_overwrite;
+ uint64_t dsu;
+ uint64_t dsw;
+ uint64_t dsunit;
+ uint64_t dswidth;
+ bool force_overwrite;
struct fsxattr fsx;
- int ilflag;
- int imaxpct;
- int imflag;
- int inodelog;
- int inopblock;
- int ipflag;
- int isflag;
- int isize;
+ bool ilflag;
+ uint64_t imaxpct;
+ bool imflag;
+ uint64_t inodelog;
+ uint64_t inopblock;
+ bool ipflag;
+ bool isflag;
+ uint64_t isize;
char *label = NULL;
- int laflag;
- int lalign;
- int ldflag;
- int liflag;
+ bool laflag;
+ uint64_t lalign;
+ bool ldflag;
+ bool liflag;
xfs_agnumber_t logagno;
xfs_rfsblock_t logblocks;
char *logfile;
- int loginternal;
+ uint64_t loginternal;
char *logsize;
xfs_fsblock_t logstart;
- int lvflag;
- int lsflag;
- int lsuflag;
- int lsunitflag;
- int lsectorlog;
- int lsectorsize;
- int lslflag;
- int lssflag;
- int lsu;
- int lsunit;
- int min_logblocks;
+ bool lvflag;
+ bool lsflag;
+ bool lsuflag;
+ bool lsunitflag;
+ uint64_t lsectorlog;
+ uint64_t lsectorsize;
+ bool lslflag;
+ bool lssflag;
+ uint64_t lsu;
+ uint64_t lsunit;
+ uint64_t min_logblocks;
xfs_mount_t *mp;
xfs_mount_t mbuf;
xfs_extlen_t nbmblocks;
- int nlflag;
- int nodsflag;
- int norsflag;
+ bool nlflag;
+ bool nodsflag;
+ bool norsflag;
xfs_alloc_rec_t *nrec;
- int nsflag;
- int nvflag;
- int Nflag;
- int discard = 1;
+ bool nsflag;
+ bool nvflag;
+ bool Nflag;
+ uint64_t discard = 1;
char *p;
char *protofile;
char *protostring;
- int qflag;
+ bool qflag;
xfs_rfsblock_t rtblocks;
xfs_extlen_t rtextblocks;
xfs_rtblock_t rtextents;
@@ -1492,13 +1492,13 @@ main(
char *rtfile;
char *rtsize;
xfs_sb_t *sbp;
- int sectorlog;
- __uint64_t sector_mask;
- int slflag;
- int ssflag;
- __uint64_t tmp_agsize;
+ uint64_t sectorlog;
+ uint64_t sector_mask;
+ bool slflag;
+ bool ssflag;
+ uint64_t tmp_agsize;
uuid_t uuid;
- int worst_freelist;
+ uint64_t worst_freelist;
libxfs_init_t xi;
struct fs_topology ft;
struct sb_feat_args sb_feat = {
@@ -1528,20 +1528,20 @@ main(
blocklog = blocksize = 0;
sectorlog = lsectorlog = 0;
sectorsize = lsectorsize = 0;
- agsize = daflag = dasize = dblocks = 0;
- ilflag = imflag = ipflag = isflag = 0;
- liflag = laflag = lsflag = lsuflag = lsunitflag = ldflag = lvflag = 0;
+ agsize = dasize = dblocks = 0;
+ daflag = ilflag = imflag = ipflag = isflag = false;
+ liflag = laflag = lsflag = lsuflag = lsunitflag = ldflag = lvflag = false;
loginternal = 1;
logagno = logblocks = rtblocks = rtextblocks = 0;
- Nflag = nlflag = nsflag = nvflag = 0;
+ Nflag = nlflag = nsflag = nvflag = false;
dirblocklog = dirblocksize = 0;
- qflag = 0;
+ qflag = false;
imaxpct = inodelog = inopblock = isize = 0;
dfile = logfile = rtfile = NULL;
dsize = logsize = rtsize = rtextsize = protofile = NULL;
dsu = dsw = dsunit = dswidth = lalign = lsu = lsunit = 0;
- nodsflag = norsflag = 0;
- force_overwrite = 0;
+ nodsflag = norsflag = false;
+ force_overwrite = false;
worst_freelist = 0;
memset(&fsx, 0, sizeof(fsx));
@@ -1553,7 +1553,7 @@ main(
switch (c) {
case 'C':
case 'f':
- force_overwrite = 1;
+ force_overwrite = true;
break;
case 'b':
p = optarg;
@@ -1962,7 +1962,7 @@ main(
blocksize = 1 << XFS_DFL_BLOCKSIZE_LOG;
}
if (blocksize < XFS_MIN_BLOCKSIZE || blocksize > XFS_MAX_BLOCKSIZE) {
- fprintf(stderr, _("illegal block size %d\n"), blocksize);
+ fprintf(stderr, _("illegal block size %"PRIu64"\n"), blocksize);
usage();
}
if (sb_feat.crcs_enabled && blocksize < XFS_MIN_CRC_BLOCKSIZE) {
@@ -2026,7 +2026,7 @@ _("Minimum block size for CRC enabled filesystems is %d bytes.\n"),
if ((blocksize < sectorsize) && (blocksize >= ft.lsectorsize)) {
fprintf(stderr,
-_("specified blocksize %d is less than device physical sector size %d\n"),
+_("specified blocksize %"PRIu64" is less than device physical sector size %d\n"),
blocksize, ft.psectorsize);
fprintf(stderr,
_("switching to logical sector size %d\n"),
@@ -2047,21 +2047,21 @@ _("switching to logical sector size %d\n"),
if (sectorsize < XFS_MIN_SECTORSIZE ||
sectorsize > XFS_MAX_SECTORSIZE || sectorsize > blocksize) {
if (ssflag)
- fprintf(stderr, _("illegal sector size %d\n"), sectorsize);
+ fprintf(stderr, _("illegal sector size %"PRIu64"\n"), sectorsize);
else
fprintf(stderr,
-_("block size %d cannot be smaller than logical sector size %d\n"),
+_("block size %"PRIu64" cannot be smaller than logical sector size %d\n"),
blocksize, ft.lsectorsize);
usage();
}
if (sectorsize < ft.lsectorsize) {
- fprintf(stderr, _("illegal sector size %d; hw sector is %d\n"),
+ fprintf(stderr, _("illegal sector size %"PRIu64"; hw sector is %d\n"),
sectorsize, ft.lsectorsize);
usage();
}
if (lsectorsize < XFS_MIN_SECTORSIZE ||
lsectorsize > XFS_MAX_SECTORSIZE || lsectorsize > blocksize) {
- fprintf(stderr, _("illegal log sector size %d\n"), lsectorsize);
+ fprintf(stderr, _("illegal log sector size %"PRIu64"\n"), lsectorsize);
usage();
} else if (lsectorsize > XFS_MIN_SECTORSIZE && !lsu && !lsunit) {
lsu = blocksize;
@@ -2167,7 +2167,7 @@ _("rmapbt not supported with realtime devices\n"));
if (nsflag || nlflag) {
if (dirblocksize < blocksize ||
dirblocksize > XFS_MAX_BLOCKSIZE) {
- fprintf(stderr, _("illegal directory block size %d\n"),
+ fprintf(stderr, _("illegal directory block size %"PRIu64"\n"),
dirblocksize);
usage();
}
@@ -2181,7 +2181,7 @@ _("rmapbt not supported with realtime devices\n"));
if (dsize) {
- __uint64_t dbytes;
+ uint64_t dbytes;
dbytes = getnum(dsize, &dopts, D_SIZE);
if (dbytes % XFS_MIN_BLOCKSIZE) {
@@ -2193,7 +2193,7 @@ _("rmapbt not supported with realtime devices\n"));
dblocks = (xfs_rfsblock_t)(dbytes >> blocklog);
if (dbytes % blocksize)
fprintf(stderr, _("warning: "
- "data length %lld not a multiple of %d, truncated to %lld\n"),
+ "data length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
(long long)dbytes, blocksize,
(long long)(dblocks << blocklog));
}
@@ -2213,7 +2213,7 @@ _("rmapbt not supported with realtime devices\n"));
}
if (logsize) {
- __uint64_t logbytes;
+ uint64_t logbytes;
logbytes = getnum(logsize, &lopts, L_SIZE);
if (logbytes % XFS_MIN_BLOCKSIZE) {
@@ -2225,12 +2225,12 @@ _("rmapbt not supported with realtime devices\n"));
logblocks = (xfs_rfsblock_t)(logbytes >> blocklog);
if (logbytes % blocksize)
fprintf(stderr,
- _("warning: log length %lld not a multiple of %d, truncated to %lld\n"),
+ _("warning: log length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
(long long)logbytes, blocksize,
(long long)(logblocks << blocklog));
}
if (rtsize) {
- __uint64_t rtbytes;
+ uint64_t rtbytes;
rtbytes = getnum(rtsize, &ropts, R_SIZE);
if (rtbytes % XFS_MIN_BLOCKSIZE) {
@@ -2242,7 +2242,7 @@ _("rmapbt not supported with realtime devices\n"));
rtblocks = (xfs_rfsblock_t)(rtbytes >> blocklog);
if (rtbytes % blocksize)
fprintf(stderr,
- _("warning: rt length %lld not a multiple of %d, truncated to %lld\n"),
+ _("warning: rt length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
(long long)rtbytes, blocksize,
(long long)(rtblocks << blocklog));
}
@@ -2250,12 +2250,12 @@ _("rmapbt not supported with realtime devices\n"));
* If specified, check rt extent size against its constraints.
*/
if (rtextsize) {
- __uint64_t rtextbytes;
+ uint64_t rtextbytes;
rtextbytes = getnum(rtextsize, &ropts, R_EXTSIZE);
if (rtextbytes % blocksize) {
fprintf(stderr,
- _("illegal rt extent size %lld, not a multiple of %d\n"),
+ _("illegal rt extent size %lld, not a multiple of %"PRIu64"\n"),
(long long)rtextbytes, blocksize);
usage();
}
@@ -2266,8 +2266,8 @@ _("rmapbt not supported with realtime devices\n"));
* and the underlying volume is striped, then set rtextblocks
* to the stripe width.
*/
- __uint64_t rswidth;
- __uint64_t rtextbytes;
+ uint64_t rswidth;
+ uint64_t rtextbytes;
if (!norsflag && !xi.risfile && !(!rtsize && xi.disfile))
rswidth = ft.rtswidth;
@@ -2298,16 +2298,16 @@ _("rmapbt not supported with realtime devices\n"));
isize > XFS_DINODE_MAX_SIZE) {
int maxsz;
- fprintf(stderr, _("illegal inode size %d\n"), isize);
+ fprintf(stderr, _("illegal inode size %"PRIu64"\n"), isize);
maxsz = MIN(blocksize / XFS_MIN_INODE_PERBLOCK,
XFS_DINODE_MAX_SIZE);
if (XFS_DINODE_MIN_SIZE == maxsz)
fprintf(stderr,
- _("allowable inode size with %d byte blocks is %d\n"),
+ _("allowable inode size with %"PRIu64" byte blocks is %d\n"),
blocksize, XFS_DINODE_MIN_SIZE);
else
fprintf(stderr,
- _("allowable inode size with %d byte blocks is between %d and %d\n"),
+ _("allowable inode size with %"PRIu64" byte blocks is between %d and %d\n"),
blocksize, XFS_DINODE_MIN_SIZE, maxsz);
exit(1);
}
@@ -2345,10 +2345,10 @@ _("rmapbt not supported with realtime devices\n"));
* multiple of the sector size, or 1024, whichever is larger.
*/
- sector_mask = (__uint64_t)-1 << (MAX(sectorlog, 10) - BBSHIFT);
+ sector_mask = (uint64_t)-1 << (MAX(sectorlog, 10) - BBSHIFT);
xi.dsize &= sector_mask;
xi.rtsize &= sector_mask;
- xi.logBBsize &= (__uint64_t)-1 << (MAX(lsectorlog, 10) - BBSHIFT);
+ xi.logBBsize &= (uint64_t)-1 << (MAX(lsectorlog, 10) - BBSHIFT);
/* don't do discards on print-only runs or on files */
@@ -2411,19 +2411,19 @@ _("rmapbt not supported with realtime devices\n"));
if (xi.dbsize > sectorsize) {
fprintf(stderr, _(
-"Warning: the data subvolume sector size %u is less than the sector size \n\
+"Warning: the data subvolume sector size %"PRIu64" is less than the sector size \n\
reported by the device (%u).\n"),
sectorsize, xi.dbsize);
}
if (!loginternal && xi.lbsize > lsectorsize) {
fprintf(stderr, _(
-"Warning: the log subvolume sector size %u is less than the sector size\n\
+"Warning: the log subvolume sector size %"PRIu64" is less than the sector size\n\
reported by the device (%u).\n"),
lsectorsize, xi.lbsize);
}
if (rtsize && xi.rtsize > 0 && xi.rtbsize > sectorsize) {
fprintf(stderr, _(
-"Warning: the realtime subvolume sector size %u is less than the sector size\n\
+"Warning: the realtime subvolume sector size %"PRIu64" is less than the sector size\n\
reported by the device (%u).\n"),
sectorsize, xi.rtbsize);
}
@@ -2453,14 +2453,14 @@ reported by the device (%u).\n"),
if (dsunit) {
if (ft.dsunit && ft.dsunit != dsunit) {
fprintf(stderr,
- _("%s: Specified data stripe unit %d "
+ _("%s: Specified data stripe unit %"PRIu64" "
"is not the same as the volume stripe "
"unit %d\n"),
progname, dsunit, ft.dsunit);
}
if (ft.dswidth && ft.dswidth != dswidth) {
fprintf(stderr,
- _("%s: Specified data stripe width %d "
+ _("%s: Specified data stripe width %"PRIu64" "
"is not the same as the volume stripe "
"width %d\n"),
progname, dswidth, ft.dswidth);
@@ -2478,7 +2478,7 @@ reported by the device (%u).\n"),
*/
if (agsize % blocksize) {
fprintf(stderr,
- _("agsize (%lld) not a multiple of fs blk size (%d)\n"),
+ _("agsize (%lld) not a multiple of fs blk size (%"PRIu64")\n"),
(long long)agsize, blocksize);
usage();
}
@@ -2528,7 +2528,7 @@ reported by the device (%u).\n"),
(dblocks % agsize != 0);
if (dasize)
fprintf(stderr,
- _("agsize rounded to %lld, swidth = %d\n"),
+ _("agsize rounded to %lld, swidth = %"PRIu64"\n"),
(long long)agsize, dswidth);
} else {
if (nodsflag) {
@@ -2585,8 +2585,8 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
dsunit = dswidth = 0;
else {
fprintf(stderr,
- _("%s: Stripe unit(%d) or stripe width(%d) is "
- "not a multiple of the block size(%d)\n"),
+ _("%s: Stripe unit(%"PRIu64") or stripe width(%"PRIu64") is "
+ "not a multiple of the block size(%"PRIu64")\n"),
progname, BBTOB(dsunit), BBTOB(dswidth),
blocksize);
exit(1);
@@ -2626,7 +2626,7 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
/* Warn only if specified on commandline */
if (lsuflag || lsunitflag) {
fprintf(stderr,
- _("log stripe unit (%d bytes) is too large (maximum is 256KiB)\n"),
+ _("log stripe unit (%"PRIu64" bytes) is too large (maximum is 256KiB)\n"),
(lsunit * blocksize));
fprintf(stderr,
_("log stripe unit adjusted to 32KiB\n"));
@@ -2771,14 +2771,14 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
if (!qflag || Nflag) {
printf(_(
- "meta-data=%-22s isize=%-6d agcount=%lld, agsize=%lld blks\n"
- " =%-22s sectsz=%-5u attr=%u, projid32bit=%u\n"
+ "meta-data=%-22s isize=%-6lu agcount=%lld, agsize=%lld blks\n"
+ " =%-22s sectsz=%-5lu attr=%u, projid32bit=%u\n"
" =%-22s crc=%-8u finobt=%u, sparse=%u, rmapbt=%u, reflink=%u\n"
- "data =%-22s bsize=%-6u blocks=%llu, imaxpct=%u\n"
- " =%-22s sunit=%-6u swidth=%u blks\n"
- "naming =version %-14u bsize=%-6u ascii-ci=%d ftype=%d\n"
+ "data =%-22s bsize=%-6lu blocks=%llu, imaxpct=%"PRIu64"\n"
+ " =%-22s sunit=%-6lu swidth=%"PRIu64" blks\n"
+ "naming =version %-14u bsize=%-6lu ascii-ci=%d ftype=%d\n"
"log =%-22s bsize=%-6d blocks=%lld, version=%d\n"
- " =%-22s sectsz=%-5u sunit=%d blks, lazy-count=%d\n"
+ " =%-22s sectsz=%-5lu sunit=%"PRIu64" blks, lazy-count=%d\n"
"realtime =%-22s extsz=%-6d blocks=%lld, rtextents=%lld\n"),
dfile, isize, (long long)agcount, (long long)agsize,
"", sectorsize, sb_feat.attr_version,
@@ -3431,17 +3431,17 @@ unknown(
usage();
}
-long long
+uint64_t
cvtnum(
unsigned int blksize,
unsigned int sectsize,
const char *s)
{
- long long i;
+ uint64_t i;
char *sp;
int c;
- i = strtoll(s, &sp, 0);
+ i = strtoull(s, &sp, 0);
if (i == 0 && sp == s)
return -1LL;
if (*sp == '\0')
--
2.1.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2 v2] mkfs: remove long long type casts
2017-04-19 15:30 ` [PATCH 2/2] mkfs: remove long long type casts Jan Tulak
@ 2017-04-20 8:33 ` Jan Tulak
2017-04-25 1:40 ` [PATCH 2/2] " Luis R. Rodriguez
1 sibling, 0 replies; 17+ messages in thread
From: Jan Tulak @ 2017-04-20 8:33 UTC (permalink / raw)
To: linux-xfs; +Cc: Jan Tulak
We have uint64_T, why cast it to long long for prints? Just print it as
it is.
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
EDIT:
* __uint64_t -> uint64_t
* convert %lu to PRIu64
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
mkfs/xfs_mkfs.c | 148 ++++++++++++++++++++++++++++----------------------------
1 file changed, 74 insertions(+), 74 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 40a32be..83a04fc 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -916,9 +916,9 @@ fixup_log_stripe_unit(
}
*logblocks = tmp_logblocks;
} else {
- fprintf(stderr, _("log size %lld is not a multiple "
+ fprintf(stderr, _("log size %"PRIu64" is not a multiple "
"of the log stripe unit %d\n"),
- (long long) *logblocks, sunit);
+ *logblocks, sunit);
usage();
}
}
@@ -945,7 +945,7 @@ fixup_internal_log_stripe(
if (*logblocks > agsize - XFS_FSB_TO_AGBNO(mp, logstart)) {
fprintf(stderr,
_("Due to stripe alignment, the internal log size "
- "(%lld) is too large.\n"), (long long) *logblocks);
+ "(%"PRIu64") is too large.\n"), *logblocks);
fprintf(stderr, _("Must fit within an allocation group.\n"));
usage();
}
@@ -957,20 +957,20 @@ validate_log_size(uint64_t logblocks, int blocklog, int min_logblocks)
{
if (logblocks < min_logblocks) {
fprintf(stderr,
- _("log size %lld blocks too small, minimum size is %d blocks\n"),
- (long long)logblocks, min_logblocks);
+ _("log size %"PRIu64" blocks too small, minimum size is %d blocks\n"),
+ logblocks, min_logblocks);
usage();
}
if (logblocks > XFS_MAX_LOG_BLOCKS) {
fprintf(stderr,
- _("log size %lld blocks too large, maximum size is %lld blocks\n"),
- (long long)logblocks, XFS_MAX_LOG_BLOCKS);
+ _("log size %"PRIu64" blocks too large, maximum size is %lld blocks\n"),
+ logblocks, XFS_MAX_LOG_BLOCKS);
usage();
}
if ((logblocks << blocklog) > XFS_MAX_LOG_BYTES) {
fprintf(stderr,
- _("log size %lld bytes too large, maximum size is %lld bytes\n"),
- (long long)(logblocks << blocklog), XFS_MAX_LOG_BYTES);
+ _("log size %"PRIu64" bytes too large, maximum size is %lld bytes\n"),
+ (logblocks << blocklog), XFS_MAX_LOG_BYTES);
usage();
}
}
@@ -1006,43 +1006,43 @@ validate_ag_geometry(
{
if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
fprintf(stderr,
- _("agsize (%lld blocks) too small, need at least %lld blocks\n"),
- (long long)agsize,
- (long long)XFS_AG_MIN_BLOCKS(blocklog));
+ _("agsize (%"PRIu64" blocks) too small, need at least %"PRIu64" blocks\n"),
+ agsize,
+ (uint64_t)XFS_AG_MIN_BLOCKS(blocklog));
usage();
}
if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
fprintf(stderr,
- _("agsize (%lld blocks) too big, maximum is %lld blocks\n"),
- (long long)agsize,
- (long long)XFS_AG_MAX_BLOCKS(blocklog));
+ _("agsize (%"PRIu64" blocks) too big, maximum is %"PRIu64" blocks\n"),
+ agsize,
+ (uint64_t)XFS_AG_MAX_BLOCKS(blocklog));
usage();
}
if (agsize > dblocks) {
fprintf(stderr,
- _("agsize (%lld blocks) too big, data area is %lld blocks\n"),
- (long long)agsize, (long long)dblocks);
+ _("agsize (%"PRIu64" blocks) too big, data area is %"PRIu64" blocks\n"),
+ agsize, dblocks);
usage();
}
if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
fprintf(stderr,
- _("too many allocation groups for size = %lld\n"),
- (long long)agsize);
- fprintf(stderr, _("need at most %lld allocation groups\n"),
- (long long)(dblocks / XFS_AG_MIN_BLOCKS(blocklog) +
+ _("too many allocation groups for size = %"PRIu64"\n"),
+ agsize);
+ fprintf(stderr, _("need at most %"PRIu64" allocation groups\n"),
+ (uint64_t) (dblocks / XFS_AG_MIN_BLOCKS(blocklog) +
(dblocks % XFS_AG_MIN_BLOCKS(blocklog) != 0)));
usage();
}
if (agsize > XFS_AG_MAX_BLOCKS(blocklog)) {
fprintf(stderr,
- _("too few allocation groups for size = %lld\n"), (long long)agsize);
+ _("too few allocation groups for size = %"PRIu64"\n"), agsize);
fprintf(stderr,
- _("need at least %lld allocation groups\n"),
- (long long)(dblocks / XFS_AG_MAX_BLOCKS(blocklog) +
+ _("need at least %"PRIu64" allocation groups\n"),
+ (uint64_t)(dblocks / XFS_AG_MAX_BLOCKS(blocklog) +
(dblocks % XFS_AG_MAX_BLOCKS(blocklog) != 0)));
usage();
}
@@ -1054,9 +1054,9 @@ validate_ag_geometry(
if ( dblocks % agsize != 0 &&
(dblocks % agsize < XFS_AG_MIN_BLOCKS(blocklog))) {
fprintf(stderr,
- _("last AG size %lld blocks too small, minimum size is %lld blocks\n"),
- (long long)(dblocks % agsize),
- (long long)XFS_AG_MIN_BLOCKS(blocklog));
+ _("last AG size %"PRIu64" blocks too small, minimum size is %"PRIu64" blocks\n"),
+ (dblocks % agsize),
+ (uint64_t)XFS_AG_MIN_BLOCKS(blocklog));
usage();
}
@@ -1065,8 +1065,8 @@ validate_ag_geometry(
*/
if (agcount > XFS_MAX_AGNUMBER + 1) {
fprintf(stderr,
- _("%lld allocation groups is too many, maximum is %lld\n"),
- (long long)agcount, (long long)XFS_MAX_AGNUMBER + 1);
+ _("%"PRIu64" allocation groups is too many, maximum is %"PRIu64"\n"),
+ agcount, (uint64_t)XFS_MAX_AGNUMBER + 1);
usage();
}
}
@@ -2186,16 +2186,16 @@ _("rmapbt not supported with realtime devices\n"));
dbytes = getnum(dsize, &dopts, D_SIZE);
if (dbytes % XFS_MIN_BLOCKSIZE) {
fprintf(stderr,
- _("illegal data length %lld, not a multiple of %d\n"),
- (long long)dbytes, XFS_MIN_BLOCKSIZE);
+ _("illegal data length %"PRIu64", not a multiple of %d\n"),
+ dbytes, XFS_MIN_BLOCKSIZE);
usage();
}
dblocks = (xfs_rfsblock_t)(dbytes >> blocklog);
if (dbytes % blocksize)
fprintf(stderr, _("warning: "
- "data length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
- (long long)dbytes, blocksize,
- (long long)(dblocks << blocklog));
+ "data length %"PRIu64" not a multiple of %"PRIu64", truncated to %"PRIu64"\n"),
+ dbytes, blocksize,
+ (uint64_t)(dblocks << blocklog));
}
if (ipflag) {
inodelog = blocklog - libxfs_highbit32(inopblock);
@@ -2218,16 +2218,16 @@ _("rmapbt not supported with realtime devices\n"));
logbytes = getnum(logsize, &lopts, L_SIZE);
if (logbytes % XFS_MIN_BLOCKSIZE) {
fprintf(stderr,
- _("illegal log length %lld, not a multiple of %d\n"),
- (long long)logbytes, XFS_MIN_BLOCKSIZE);
+ _("illegal log length %"PRIu64", not a multiple of %d\n"),
+ logbytes, XFS_MIN_BLOCKSIZE);
usage();
}
logblocks = (xfs_rfsblock_t)(logbytes >> blocklog);
if (logbytes % blocksize)
fprintf(stderr,
- _("warning: log length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
- (long long)logbytes, blocksize,
- (long long)(logblocks << blocklog));
+ _("warning: log length %"PRIu64" not a multiple of %"PRIu64", truncated to %"PRIu64"\n"),
+ logbytes, blocksize,
+ (uint64_t)(logblocks << blocklog));
}
if (rtsize) {
uint64_t rtbytes;
@@ -2235,16 +2235,16 @@ _("rmapbt not supported with realtime devices\n"));
rtbytes = getnum(rtsize, &ropts, R_SIZE);
if (rtbytes % XFS_MIN_BLOCKSIZE) {
fprintf(stderr,
- _("illegal rt length %lld, not a multiple of %d\n"),
- (long long)rtbytes, XFS_MIN_BLOCKSIZE);
+ _("illegal rt length %"PRIu64", not a multiple of %d\n"),
+ rtbytes, XFS_MIN_BLOCKSIZE);
usage();
}
rtblocks = (xfs_rfsblock_t)(rtbytes >> blocklog);
if (rtbytes % blocksize)
fprintf(stderr,
- _("warning: rt length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
- (long long)rtbytes, blocksize,
- (long long)(rtblocks << blocklog));
+ _("warning: rt length %"PRIu64" not a multiple of %"PRIu64", truncated to %"PRIu64"\n"),
+ rtbytes, blocksize,
+ (uint64_t)(rtblocks << blocklog));
}
/*
* If specified, check rt extent size against its constraints.
@@ -2255,8 +2255,8 @@ _("rmapbt not supported with realtime devices\n"));
rtextbytes = getnum(rtextsize, &ropts, R_EXTSIZE);
if (rtextbytes % blocksize) {
fprintf(stderr,
- _("illegal rt extent size %lld, not a multiple of %"PRIu64"\n"),
- (long long)rtextbytes, blocksize);
+ _("illegal rt extent size %"PRIu64", not a multiple of %"PRIu64"\n"),
+ rtextbytes, blocksize);
usage();
}
rtextblocks = (xfs_extlen_t)(rtextbytes >> blocklog);
@@ -2383,8 +2383,8 @@ _("rmapbt not supported with realtime devices\n"));
if (dsize && xi.dsize > 0 && dblocks > DTOBT(xi.dsize)) {
fprintf(stderr,
_("size %s specified for data subvolume is too large, "
- "maximum is %lld blocks\n"),
- dsize, (long long)DTOBT(xi.dsize));
+ "maximum is %"PRIu64" blocks\n"),
+ dsize, (uint64_t)DTOBT(xi.dsize));
usage();
} else if (!dsize && xi.dsize > 0)
dblocks = DTOBT(xi.dsize);
@@ -2394,8 +2394,8 @@ _("rmapbt not supported with realtime devices\n"));
}
if (dblocks < XFS_MIN_DATA_BLOCKS) {
fprintf(stderr,
- _("size %lld of data subvolume is too small, minimum %d blocks\n"),
- (long long)dblocks, XFS_MIN_DATA_BLOCKS);
+ _("size %"PRIu64" of data subvolume is too small, minimum %d blocks\n"),
+ dblocks, XFS_MIN_DATA_BLOCKS);
usage();
}
@@ -2431,8 +2431,8 @@ reported by the device (%u).\n"),
if (rtsize && xi.rtsize > 0 && rtblocks > DTOBT(xi.rtsize)) {
fprintf(stderr,
_("size %s specified for rt subvolume is too large, "
- "maximum is %lld blocks\n"),
- rtsize, (long long)DTOBT(xi.rtsize));
+ "maximum is %"PRIu64" blocks\n"),
+ rtsize, (uint64_t)DTOBT(xi.rtsize));
usage();
} else if (!rtsize && xi.rtsize > 0)
rtblocks = DTOBT(xi.rtsize);
@@ -2478,8 +2478,8 @@ reported by the device (%u).\n"),
*/
if (agsize % blocksize) {
fprintf(stderr,
- _("agsize (%lld) not a multiple of fs blk size (%"PRIu64")\n"),
- (long long)agsize, blocksize);
+ _("agsize (%"PRIu64") not a multiple of fs blk size (%"PRIu64")\n"),
+ agsize, blocksize);
usage();
}
agsize /= blocksize;
@@ -2528,8 +2528,8 @@ reported by the device (%u).\n"),
(dblocks % agsize != 0);
if (dasize)
fprintf(stderr,
- _("agsize rounded to %lld, swidth = %"PRIu64"\n"),
- (long long)agsize, dswidth);
+ _("agsize rounded to %"PRIu64", swidth = %"PRIu64"\n"),
+ agsize, dswidth);
} else {
if (nodsflag) {
dsunit = dswidth = 0;
@@ -2562,8 +2562,8 @@ reported by the device (%u).\n"),
fprintf(stderr, _(
"Warning: AG size is a multiple of stripe width. This can cause performance\n\
problems by aligning all AGs on the same disk. To avoid this, run mkfs with\n\
-an AG size that is one stripe unit smaller, for example %llu.\n"),
- (unsigned long long)tmp_agsize);
+an AG size that is one stripe unit smaller, for example %"PRIu64".\n"),
+ tmp_agsize);
} else {
agsize = tmp_agsize;
agcount = dblocks/agsize + (dblocks % agsize != 0);
@@ -2645,8 +2645,8 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
min_logblocks = MAX(min_logblocks, XFS_MIN_LOG_BYTES>>blocklog);
if (logsize && xi.logBBsize > 0 && logblocks > DTOBT(xi.logBBsize)) {
fprintf(stderr,
-_("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
- logsize, (long long)DTOBT(xi.logBBsize));
+_("size %s specified for log subvolume is too large, maximum is %"PRIu64" blocks\n"),
+ logsize, (uint64_t)DTOBT(xi.logBBsize));
usage();
} else if (!logsize && xi.logBBsize > 0) {
logblocks = DTOBT(xi.logBBsize);
@@ -2655,8 +2655,8 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
_("size specified for non-existent log subvolume\n"));
usage();
} else if (loginternal && logsize && logblocks >= dblocks) {
- fprintf(stderr, _("size %lld too large for internal log\n"),
- (long long)logblocks);
+ fprintf(stderr, _("size %"PRIu64" too large for internal log\n"),
+ logblocks);
usage();
} else if (!loginternal && !xi.logdev) {
logblocks = 0;
@@ -2733,16 +2733,16 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
}
if (logblocks > agsize - libxfs_prealloc_blocks(mp)) {
fprintf(stderr,
- _("internal log size %lld too large, must fit in allocation group\n"),
- (long long)logblocks);
+ _("internal log size %"PRIu64" too large, must fit in allocation group\n"),
+ logblocks);
usage();
}
if (laflag) {
if (logagno >= agcount) {
fprintf(stderr,
- _("log ag number %d too large, must be less than %lld\n"),
- logagno, (long long)agcount);
+ _("log ag number %d too large, must be less than %"PRIu64"\n"),
+ logagno, agcount);
usage();
}
} else
@@ -2771,29 +2771,29 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
if (!qflag || Nflag) {
printf(_(
- "meta-data=%-22s isize=%-6lu agcount=%lld, agsize=%lld blks\n"
+ "meta-data=%-22s isize=%-6lu agcount=%"PRIu64", agsize=%"PRIu64" blks\n"
" =%-22s sectsz=%-5lu attr=%u, projid32bit=%u\n"
" =%-22s crc=%-8u finobt=%u, sparse=%u, rmapbt=%u, reflink=%u\n"
- "data =%-22s bsize=%-6lu blocks=%llu, imaxpct=%"PRIu64"\n"
+ "data =%-22s bsize=%-6lu blocks=%"PRIu64", imaxpct=%"PRIu64"\n"
" =%-22s sunit=%-6lu swidth=%"PRIu64" blks\n"
"naming =version %-14u bsize=%-6lu ascii-ci=%d ftype=%d\n"
- "log =%-22s bsize=%-6d blocks=%lld, version=%d\n"
+ "log =%-22s bsize=%-6d blocks=%"PRIu64", version=%d\n"
" =%-22s sectsz=%-5lu sunit=%"PRIu64" blks, lazy-count=%d\n"
- "realtime =%-22s extsz=%-6d blocks=%lld, rtextents=%lld\n"),
- dfile, isize, (long long)agcount, (long long)agsize,
+ "realtime =%-22s extsz=%-6d blocks=%"PRIu64", rtextents=%"PRIu64"\n"),
+ dfile, isize, agcount, agsize,
"", sectorsize, sb_feat.attr_version,
!sb_feat.projid16bit,
"", sb_feat.crcs_enabled, sb_feat.finobt, sb_feat.spinodes,
sb_feat.rmapbt, sb_feat.reflink,
- "", blocksize, (long long)dblocks, imaxpct,
+ "", blocksize, dblocks, imaxpct,
"", dsunit, dswidth,
sb_feat.dir_version, dirblocksize, sb_feat.nci,
sb_feat.dirftype,
- logfile, 1 << blocklog, (long long)logblocks,
+ logfile, 1 << blocklog, logblocks,
sb_feat.log_version, "", lsectorsize, lsunit,
sb_feat.lazy_sb_counters,
rtfile, rtextblocks << blocklog,
- (long long)rtblocks, (long long)rtextents);
+ rtblocks, rtextents);
if (Nflag)
exit(0);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v2] mkfs: unify numeric types of main variables in main()
2017-04-20 8:33 ` [PATCH 1/2 v2] " Jan Tulak
@ 2017-04-20 13:28 ` Luis R. Rodriguez
2017-04-20 13:57 ` Jan Tulak
2017-04-26 3:58 ` Eric Sandeen
1 sibling, 1 reply; 17+ messages in thread
From: Luis R. Rodriguez @ 2017-04-20 13:28 UTC (permalink / raw)
To: Jan Tulak; +Cc: linux-xfs
On Thu, Apr 20, 2017 at 10:33:23AM +0200, Jan Tulak wrote:
> Thus, uint64_t is the best candidate as the target
> type for numeric values. The existing uses of __uint64_t in mkfs change to the
> standard uint64_t type.
<-- snip -->
> CHANGES:
> * __uint64_t -> uint64_t
<-- snip -->
> diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h
> index ce9bbce..ab74536 100644
> --- a/include/xfs_multidisk.h
> +++ b/include/xfs_multidisk.h
> @@ -57,7 +57,7 @@
> #define XFS_NOMULTIDISK_AGLOG 2 /* 4 AGs */
> #define XFS_MULTIDISK_AGCOUNT (1 << XFS_MULTIDISK_AGLOG)
>
> -extern long long cvtnum(unsigned int blksize, unsigned int sectsize,
> +extern __uint64_t cvtnum(unsigned int blksize, unsigned int sectsize,
I thought uint64_t was desired ?
> -long long
> +uint64_t
> cvtnum(
> unsigned int blksize,
> unsigned int sectsize,
> const char *s)
> {
Indeed. Probably just a missed case?
Luis
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v2] mkfs: unify numeric types of main variables in main()
2017-04-20 13:28 ` Luis R. Rodriguez
@ 2017-04-20 13:57 ` Jan Tulak
0 siblings, 0 replies; 17+ messages in thread
From: Jan Tulak @ 2017-04-20 13:57 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linux-xfs
On Thu, Apr 20, 2017 at 3:28 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Thu, Apr 20, 2017 at 10:33:23AM +0200, Jan Tulak wrote:
>> Thus, uint64_t is the best candidate as the target
>> type for numeric values. The existing uses of __uint64_t in mkfs change to the
>> standard uint64_t type.
>
> <-- snip -->
>
>> CHANGES:
>> * __uint64_t -> uint64_t
>
> <-- snip -->
>
>> diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h
>> index ce9bbce..ab74536 100644
>> --- a/include/xfs_multidisk.h
>> +++ b/include/xfs_multidisk.h
>> @@ -57,7 +57,7 @@
>> #define XFS_NOMULTIDISK_AGLOG 2 /* 4 AGs */
>> #define XFS_MULTIDISK_AGCOUNT (1 << XFS_MULTIDISK_AGLOG)
>>
>> -extern long long cvtnum(unsigned int blksize, unsigned int sectsize,
>> +extern __uint64_t cvtnum(unsigned int blksize, unsigned int sectsize,
>
> I thought uint64_t was desired ?
>
>> -long long
>> +uint64_t
>> cvtnum(
>> unsigned int blksize,
>> unsigned int sectsize,
>> const char *s)
>> {
>
>
> Indeed. Probably just a missed case?
>
Yes, sorry for missing that.
Thanks,
Jan
> Luis
--
Jan Tulak
jtulak@redhat.com / jan@tulak.me
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2 v3] mkfs: unify numeric types of main variables in main()
2017-04-19 15:30 [PATCH 1/2] mkfs: unify numeric types of main variables in main() Jan Tulak
` (2 preceding siblings ...)
2017-04-20 8:33 ` [PATCH 1/2 v2] " Jan Tulak
@ 2017-04-20 13:58 ` Jan Tulak
2017-04-25 1:37 ` Luis R. Rodriguez
3 siblings, 1 reply; 17+ messages in thread
From: Jan Tulak @ 2017-04-20 13:58 UTC (permalink / raw)
To: linux-xfs; +Cc: Jan Tulak
In the past, when mkfs was first written, it used atoi and
similar calls, so the variables were ints. However, the situation moved
since then and in course of the time, mkfs began to use other types too.
Clean and unify it. We don't need negative values anywhere in the code and
some numbers has to be 64bit. Thus, uint64_t is the best candidate as the target
type for numeric values. The existing uses of __uint64_t in mkfs change to the
standard uint64_t type. For flag values let's use boolean.
This patch changes variables declared at the beginning of main() +
block/sectorsize, making only minimal changes. The following patch
cleans some now-unnecessary type casts.
It would be nice to change types in some of the structures too, but
this might lead to changes outside of mkfs, so I'm skipping them for
this moment to keep it simple.
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
CHANGES:
* __uint64_t -> uint64_t (now also in multidisk.h)
* Add other places, where we still used long long and I forgot them
before (cvtnum, getnum, subopt_param)
Previous changes
* Flags as bool
* %lu as PRIu64
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
include/xfs_multidisk.h | 2 +-
mkfs/xfs_mkfs.c | 266 ++++++++++++++++++++++++------------------------
2 files changed, 134 insertions(+), 134 deletions(-)
diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h
index ce9bbce..de4924f 100644
--- a/include/xfs_multidisk.h
+++ b/include/xfs_multidisk.h
@@ -57,7 +57,7 @@
#define XFS_NOMULTIDISK_AGLOG 2 /* 4 AGs */
#define XFS_MULTIDISK_AGCOUNT (1 << XFS_MULTIDISK_AGLOG)
-extern long long cvtnum(unsigned int blksize, unsigned int sectsize,
+extern uint64_t cvtnum(unsigned int blksize, unsigned int sectsize,
const char *str);
/* proto.c */
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 7a5c49f..40a32be 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -39,8 +39,8 @@ static int ispow2(unsigned int i);
* The configured block and sector sizes are defined as global variables so
* that they don't need to be passed to functions that require them.
*/
-unsigned int blocksize;
-unsigned int sectorsize;
+uint64_t blocksize;
+uint64_t sectorsize;
#define MAX_SUBOPTS 16
#define SUBOPT_NEEDS_VAL (-1LL)
@@ -138,9 +138,9 @@ struct opt_params {
bool convert;
bool is_power_2;
int conflicts[MAX_CONFLICTS];
- long long minval;
- long long maxval;
- long long defaultval;
+ uint64_t minval;
+ uint64_t maxval;
+ uint64_t defaultval;
} subopt_params[MAX_SUBOPTS];
};
@@ -724,9 +724,9 @@ struct opt_params mopts = {
},
};
-#define TERABYTES(count, blog) ((__uint64_t)(count) << (40 - (blog)))
-#define GIGABYTES(count, blog) ((__uint64_t)(count) << (30 - (blog)))
-#define MEGABYTES(count, blog) ((__uint64_t)(count) << (20 - (blog)))
+#define TERABYTES(count, blog) ((uint64_t)(count) << (40 - (blog)))
+#define GIGABYTES(count, blog) ((uint64_t)(count) << (30 - (blog)))
+#define MEGABYTES(count, blog) ((uint64_t)(count) << (20 - (blog)))
/*
* Use this macro before we have superblock and mount structure
@@ -758,9 +758,9 @@ calc_stripe_factors(
int dsectsz,
int lsu,
int lsectsz,
- int *dsunit,
- int *dswidth,
- int *lsunit)
+ uint64_t *dsunit,
+ uint64_t *dswidth,
+ uint64_t *lsunit)
{
/* Handle data sunit/swidth options */
if ((*dsunit && !*dswidth) || (!*dsunit && *dswidth)) {
@@ -785,32 +785,32 @@ calc_stripe_factors(
usage();
}
- *dsunit = (int)BTOBBT(dsu);
+ *dsunit = BTOBBT(dsu);
*dswidth = *dsunit * dsw;
}
if (*dsunit && (*dswidth % *dsunit != 0)) {
fprintf(stderr,
- _("data stripe width (%d) must be a multiple of the "
- "data stripe unit (%d)\n"), *dswidth, *dsunit);
+ _("data stripe width (%"PRIu64") must be a multiple of the "
+ "data stripe unit (%"PRIu64")\n"), *dswidth, *dsunit);
usage();
}
/* Handle log sunit options */
if (lsu)
- *lsunit = (int)BTOBBT(lsu);
+ *lsunit = BTOBBT(lsu);
/* verify if lsu/lsunit is a multiple block size */
if (lsu % blocksize != 0) {
fprintf(stderr,
-_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
+_("log stripe unit (%d) must be a multiple of the block size (%"PRIu64")\n"),
lsu, blocksize);
exit(1);
}
if ((BBTOB(*lsunit) % blocksize != 0)) {
fprintf(stderr,
-_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
+_("log stripe unit (%"PRIu64") must be a multiple of the block size (%"PRIu64")\n"),
BBTOB(*lsunit), blocksize);
exit(1);
}
@@ -897,7 +897,7 @@ fixup_log_stripe_unit(
xfs_rfsblock_t *logblocks,
int blocklog)
{
- __uint64_t tmp_logblocks;
+ uint64_t tmp_logblocks;
/*
* Make sure that the log size is a multiple of the stripe unit
@@ -929,11 +929,11 @@ fixup_internal_log_stripe(
xfs_mount_t *mp,
int lsflag,
xfs_fsblock_t logstart,
- __uint64_t agsize,
+ uint64_t agsize,
int sunit,
xfs_rfsblock_t *logblocks,
int blocklog,
- int *lalign)
+ uint64_t *lalign)
{
if ((logstart % sunit) != 0) {
logstart = ((logstart + (sunit - 1))/sunit) * sunit;
@@ -953,7 +953,7 @@ fixup_internal_log_stripe(
}
void
-validate_log_size(__uint64_t logblocks, int blocklog, int min_logblocks)
+validate_log_size(uint64_t logblocks, int blocklog, int min_logblocks)
{
if (logblocks < min_logblocks) {
fprintf(stderr,
@@ -978,7 +978,7 @@ validate_log_size(__uint64_t logblocks, int blocklog, int min_logblocks)
static int
calc_default_imaxpct(
int blocklog,
- __uint64_t dblocks)
+ uint64_t dblocks)
{
/*
* This returns the % of the disk space that is used for
@@ -1000,9 +1000,9 @@ calc_default_imaxpct(
static void
validate_ag_geometry(
int blocklog,
- __uint64_t dblocks,
- __uint64_t agsize,
- __uint64_t agcount)
+ uint64_t dblocks,
+ uint64_t agsize,
+ uint64_t agcount)
{
if (agsize < XFS_AG_MIN_BLOCKS(blocklog)) {
fprintf(stderr,
@@ -1131,8 +1131,8 @@ zero_old_xfs_structures(
i != sb.sb_blocklog)
goto done;
- if (sb.sb_dblocks > ((__uint64_t)sb.sb_agcount * sb.sb_agblocks) ||
- sb.sb_dblocks < ((__uint64_t)(sb.sb_agcount - 1) *
+ if (sb.sb_dblocks > ((uint64_t)sb.sb_agcount * sb.sb_agblocks) ||
+ sb.sb_dblocks < ((uint64_t)(sb.sb_agcount - 1) *
sb.sb_agblocks + XFS_MIN_AG_BLOCKS))
goto done;
@@ -1152,7 +1152,7 @@ done:
}
static void
-discard_blocks(dev_t dev, __uint64_t nsectors)
+discard_blocks(dev_t dev, uint64_t nsectors)
{
int fd;
@@ -1336,14 +1336,14 @@ check_opt(
}
}
-static long long
+static uint64_t
getnum(
const char *str,
struct opt_params *opts,
int index)
{
struct subopt_param *sp = &opts->subopt_params[index];
- long long c;
+ uint64_t c;
check_opt(opts, index, false);
/* empty strings might just return a default value */
@@ -1414,77 +1414,77 @@ main(
int argc,
char **argv)
{
- __uint64_t agcount;
+ uint64_t agcount;
xfs_agf_t *agf;
xfs_agi_t *agi;
xfs_agnumber_t agno;
- __uint64_t agsize;
+ uint64_t agsize;
xfs_alloc_rec_t *arec;
struct xfs_btree_block *block;
- int blflag;
- int blocklog;
- int bsflag;
- int bsize;
+ bool blflag;
+ uint64_t blocklog;
+ bool bsflag;
+ uint64_t bsize;
xfs_buf_t *buf;
- int c;
- int daflag;
- int dasize;
+ uint64_t c;
+ bool daflag;
+ uint64_t dasize;
xfs_rfsblock_t dblocks;
char *dfile;
- int dirblocklog;
- int dirblocksize;
+ uint64_t dirblocklog;
+ uint64_t dirblocksize;
char *dsize;
- int dsu;
- int dsw;
- int dsunit;
- int dswidth;
- int force_overwrite;
+ uint64_t dsu;
+ uint64_t dsw;
+ uint64_t dsunit;
+ uint64_t dswidth;
+ bool force_overwrite;
struct fsxattr fsx;
- int ilflag;
- int imaxpct;
- int imflag;
- int inodelog;
- int inopblock;
- int ipflag;
- int isflag;
- int isize;
+ bool ilflag;
+ uint64_t imaxpct;
+ bool imflag;
+ uint64_t inodelog;
+ uint64_t inopblock;
+ bool ipflag;
+ bool isflag;
+ uint64_t isize;
char *label = NULL;
- int laflag;
- int lalign;
- int ldflag;
- int liflag;
+ bool laflag;
+ uint64_t lalign;
+ bool ldflag;
+ bool liflag;
xfs_agnumber_t logagno;
xfs_rfsblock_t logblocks;
char *logfile;
- int loginternal;
+ uint64_t loginternal;
char *logsize;
xfs_fsblock_t logstart;
- int lvflag;
- int lsflag;
- int lsuflag;
- int lsunitflag;
- int lsectorlog;
- int lsectorsize;
- int lslflag;
- int lssflag;
- int lsu;
- int lsunit;
- int min_logblocks;
+ bool lvflag;
+ bool lsflag;
+ bool lsuflag;
+ bool lsunitflag;
+ uint64_t lsectorlog;
+ uint64_t lsectorsize;
+ bool lslflag;
+ bool lssflag;
+ uint64_t lsu;
+ uint64_t lsunit;
+ uint64_t min_logblocks;
xfs_mount_t *mp;
xfs_mount_t mbuf;
xfs_extlen_t nbmblocks;
- int nlflag;
- int nodsflag;
- int norsflag;
+ bool nlflag;
+ bool nodsflag;
+ bool norsflag;
xfs_alloc_rec_t *nrec;
- int nsflag;
- int nvflag;
- int Nflag;
- int discard = 1;
+ bool nsflag;
+ bool nvflag;
+ bool Nflag;
+ uint64_t discard = 1;
char *p;
char *protofile;
char *protostring;
- int qflag;
+ bool qflag;
xfs_rfsblock_t rtblocks;
xfs_extlen_t rtextblocks;
xfs_rtblock_t rtextents;
@@ -1492,13 +1492,13 @@ main(
char *rtfile;
char *rtsize;
xfs_sb_t *sbp;
- int sectorlog;
- __uint64_t sector_mask;
- int slflag;
- int ssflag;
- __uint64_t tmp_agsize;
+ uint64_t sectorlog;
+ uint64_t sector_mask;
+ bool slflag;
+ bool ssflag;
+ uint64_t tmp_agsize;
uuid_t uuid;
- int worst_freelist;
+ uint64_t worst_freelist;
libxfs_init_t xi;
struct fs_topology ft;
struct sb_feat_args sb_feat = {
@@ -1528,20 +1528,20 @@ main(
blocklog = blocksize = 0;
sectorlog = lsectorlog = 0;
sectorsize = lsectorsize = 0;
- agsize = daflag = dasize = dblocks = 0;
- ilflag = imflag = ipflag = isflag = 0;
- liflag = laflag = lsflag = lsuflag = lsunitflag = ldflag = lvflag = 0;
+ agsize = dasize = dblocks = 0;
+ daflag = ilflag = imflag = ipflag = isflag = false;
+ liflag = laflag = lsflag = lsuflag = lsunitflag = ldflag = lvflag = false;
loginternal = 1;
logagno = logblocks = rtblocks = rtextblocks = 0;
- Nflag = nlflag = nsflag = nvflag = 0;
+ Nflag = nlflag = nsflag = nvflag = false;
dirblocklog = dirblocksize = 0;
- qflag = 0;
+ qflag = false;
imaxpct = inodelog = inopblock = isize = 0;
dfile = logfile = rtfile = NULL;
dsize = logsize = rtsize = rtextsize = protofile = NULL;
dsu = dsw = dsunit = dswidth = lalign = lsu = lsunit = 0;
- nodsflag = norsflag = 0;
- force_overwrite = 0;
+ nodsflag = norsflag = false;
+ force_overwrite = false;
worst_freelist = 0;
memset(&fsx, 0, sizeof(fsx));
@@ -1553,7 +1553,7 @@ main(
switch (c) {
case 'C':
case 'f':
- force_overwrite = 1;
+ force_overwrite = true;
break;
case 'b':
p = optarg;
@@ -1962,7 +1962,7 @@ main(
blocksize = 1 << XFS_DFL_BLOCKSIZE_LOG;
}
if (blocksize < XFS_MIN_BLOCKSIZE || blocksize > XFS_MAX_BLOCKSIZE) {
- fprintf(stderr, _("illegal block size %d\n"), blocksize);
+ fprintf(stderr, _("illegal block size %"PRIu64"\n"), blocksize);
usage();
}
if (sb_feat.crcs_enabled && blocksize < XFS_MIN_CRC_BLOCKSIZE) {
@@ -2026,7 +2026,7 @@ _("Minimum block size for CRC enabled filesystems is %d bytes.\n"),
if ((blocksize < sectorsize) && (blocksize >= ft.lsectorsize)) {
fprintf(stderr,
-_("specified blocksize %d is less than device physical sector size %d\n"),
+_("specified blocksize %"PRIu64" is less than device physical sector size %d\n"),
blocksize, ft.psectorsize);
fprintf(stderr,
_("switching to logical sector size %d\n"),
@@ -2047,21 +2047,21 @@ _("switching to logical sector size %d\n"),
if (sectorsize < XFS_MIN_SECTORSIZE ||
sectorsize > XFS_MAX_SECTORSIZE || sectorsize > blocksize) {
if (ssflag)
- fprintf(stderr, _("illegal sector size %d\n"), sectorsize);
+ fprintf(stderr, _("illegal sector size %"PRIu64"\n"), sectorsize);
else
fprintf(stderr,
-_("block size %d cannot be smaller than logical sector size %d\n"),
+_("block size %"PRIu64" cannot be smaller than logical sector size %d\n"),
blocksize, ft.lsectorsize);
usage();
}
if (sectorsize < ft.lsectorsize) {
- fprintf(stderr, _("illegal sector size %d; hw sector is %d\n"),
+ fprintf(stderr, _("illegal sector size %"PRIu64"; hw sector is %d\n"),
sectorsize, ft.lsectorsize);
usage();
}
if (lsectorsize < XFS_MIN_SECTORSIZE ||
lsectorsize > XFS_MAX_SECTORSIZE || lsectorsize > blocksize) {
- fprintf(stderr, _("illegal log sector size %d\n"), lsectorsize);
+ fprintf(stderr, _("illegal log sector size %"PRIu64"\n"), lsectorsize);
usage();
} else if (lsectorsize > XFS_MIN_SECTORSIZE && !lsu && !lsunit) {
lsu = blocksize;
@@ -2167,7 +2167,7 @@ _("rmapbt not supported with realtime devices\n"));
if (nsflag || nlflag) {
if (dirblocksize < blocksize ||
dirblocksize > XFS_MAX_BLOCKSIZE) {
- fprintf(stderr, _("illegal directory block size %d\n"),
+ fprintf(stderr, _("illegal directory block size %"PRIu64"\n"),
dirblocksize);
usage();
}
@@ -2181,7 +2181,7 @@ _("rmapbt not supported with realtime devices\n"));
if (dsize) {
- __uint64_t dbytes;
+ uint64_t dbytes;
dbytes = getnum(dsize, &dopts, D_SIZE);
if (dbytes % XFS_MIN_BLOCKSIZE) {
@@ -2193,7 +2193,7 @@ _("rmapbt not supported with realtime devices\n"));
dblocks = (xfs_rfsblock_t)(dbytes >> blocklog);
if (dbytes % blocksize)
fprintf(stderr, _("warning: "
- "data length %lld not a multiple of %d, truncated to %lld\n"),
+ "data length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
(long long)dbytes, blocksize,
(long long)(dblocks << blocklog));
}
@@ -2213,7 +2213,7 @@ _("rmapbt not supported with realtime devices\n"));
}
if (logsize) {
- __uint64_t logbytes;
+ uint64_t logbytes;
logbytes = getnum(logsize, &lopts, L_SIZE);
if (logbytes % XFS_MIN_BLOCKSIZE) {
@@ -2225,12 +2225,12 @@ _("rmapbt not supported with realtime devices\n"));
logblocks = (xfs_rfsblock_t)(logbytes >> blocklog);
if (logbytes % blocksize)
fprintf(stderr,
- _("warning: log length %lld not a multiple of %d, truncated to %lld\n"),
+ _("warning: log length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
(long long)logbytes, blocksize,
(long long)(logblocks << blocklog));
}
if (rtsize) {
- __uint64_t rtbytes;
+ uint64_t rtbytes;
rtbytes = getnum(rtsize, &ropts, R_SIZE);
if (rtbytes % XFS_MIN_BLOCKSIZE) {
@@ -2242,7 +2242,7 @@ _("rmapbt not supported with realtime devices\n"));
rtblocks = (xfs_rfsblock_t)(rtbytes >> blocklog);
if (rtbytes % blocksize)
fprintf(stderr,
- _("warning: rt length %lld not a multiple of %d, truncated to %lld\n"),
+ _("warning: rt length %lld not a multiple of %"PRIu64", truncated to %lld\n"),
(long long)rtbytes, blocksize,
(long long)(rtblocks << blocklog));
}
@@ -2250,12 +2250,12 @@ _("rmapbt not supported with realtime devices\n"));
* If specified, check rt extent size against its constraints.
*/
if (rtextsize) {
- __uint64_t rtextbytes;
+ uint64_t rtextbytes;
rtextbytes = getnum(rtextsize, &ropts, R_EXTSIZE);
if (rtextbytes % blocksize) {
fprintf(stderr,
- _("illegal rt extent size %lld, not a multiple of %d\n"),
+ _("illegal rt extent size %lld, not a multiple of %"PRIu64"\n"),
(long long)rtextbytes, blocksize);
usage();
}
@@ -2266,8 +2266,8 @@ _("rmapbt not supported with realtime devices\n"));
* and the underlying volume is striped, then set rtextblocks
* to the stripe width.
*/
- __uint64_t rswidth;
- __uint64_t rtextbytes;
+ uint64_t rswidth;
+ uint64_t rtextbytes;
if (!norsflag && !xi.risfile && !(!rtsize && xi.disfile))
rswidth = ft.rtswidth;
@@ -2298,16 +2298,16 @@ _("rmapbt not supported with realtime devices\n"));
isize > XFS_DINODE_MAX_SIZE) {
int maxsz;
- fprintf(stderr, _("illegal inode size %d\n"), isize);
+ fprintf(stderr, _("illegal inode size %"PRIu64"\n"), isize);
maxsz = MIN(blocksize / XFS_MIN_INODE_PERBLOCK,
XFS_DINODE_MAX_SIZE);
if (XFS_DINODE_MIN_SIZE == maxsz)
fprintf(stderr,
- _("allowable inode size with %d byte blocks is %d\n"),
+ _("allowable inode size with %"PRIu64" byte blocks is %d\n"),
blocksize, XFS_DINODE_MIN_SIZE);
else
fprintf(stderr,
- _("allowable inode size with %d byte blocks is between %d and %d\n"),
+ _("allowable inode size with %"PRIu64" byte blocks is between %d and %d\n"),
blocksize, XFS_DINODE_MIN_SIZE, maxsz);
exit(1);
}
@@ -2345,10 +2345,10 @@ _("rmapbt not supported with realtime devices\n"));
* multiple of the sector size, or 1024, whichever is larger.
*/
- sector_mask = (__uint64_t)-1 << (MAX(sectorlog, 10) - BBSHIFT);
+ sector_mask = (uint64_t)-1 << (MAX(sectorlog, 10) - BBSHIFT);
xi.dsize &= sector_mask;
xi.rtsize &= sector_mask;
- xi.logBBsize &= (__uint64_t)-1 << (MAX(lsectorlog, 10) - BBSHIFT);
+ xi.logBBsize &= (uint64_t)-1 << (MAX(lsectorlog, 10) - BBSHIFT);
/* don't do discards on print-only runs or on files */
@@ -2411,19 +2411,19 @@ _("rmapbt not supported with realtime devices\n"));
if (xi.dbsize > sectorsize) {
fprintf(stderr, _(
-"Warning: the data subvolume sector size %u is less than the sector size \n\
+"Warning: the data subvolume sector size %"PRIu64" is less than the sector size \n\
reported by the device (%u).\n"),
sectorsize, xi.dbsize);
}
if (!loginternal && xi.lbsize > lsectorsize) {
fprintf(stderr, _(
-"Warning: the log subvolume sector size %u is less than the sector size\n\
+"Warning: the log subvolume sector size %"PRIu64" is less than the sector size\n\
reported by the device (%u).\n"),
lsectorsize, xi.lbsize);
}
if (rtsize && xi.rtsize > 0 && xi.rtbsize > sectorsize) {
fprintf(stderr, _(
-"Warning: the realtime subvolume sector size %u is less than the sector size\n\
+"Warning: the realtime subvolume sector size %"PRIu64" is less than the sector size\n\
reported by the device (%u).\n"),
sectorsize, xi.rtbsize);
}
@@ -2453,14 +2453,14 @@ reported by the device (%u).\n"),
if (dsunit) {
if (ft.dsunit && ft.dsunit != dsunit) {
fprintf(stderr,
- _("%s: Specified data stripe unit %d "
+ _("%s: Specified data stripe unit %"PRIu64" "
"is not the same as the volume stripe "
"unit %d\n"),
progname, dsunit, ft.dsunit);
}
if (ft.dswidth && ft.dswidth != dswidth) {
fprintf(stderr,
- _("%s: Specified data stripe width %d "
+ _("%s: Specified data stripe width %"PRIu64" "
"is not the same as the volume stripe "
"width %d\n"),
progname, dswidth, ft.dswidth);
@@ -2478,7 +2478,7 @@ reported by the device (%u).\n"),
*/
if (agsize % blocksize) {
fprintf(stderr,
- _("agsize (%lld) not a multiple of fs blk size (%d)\n"),
+ _("agsize (%lld) not a multiple of fs blk size (%"PRIu64")\n"),
(long long)agsize, blocksize);
usage();
}
@@ -2528,7 +2528,7 @@ reported by the device (%u).\n"),
(dblocks % agsize != 0);
if (dasize)
fprintf(stderr,
- _("agsize rounded to %lld, swidth = %d\n"),
+ _("agsize rounded to %lld, swidth = %"PRIu64"\n"),
(long long)agsize, dswidth);
} else {
if (nodsflag) {
@@ -2585,8 +2585,8 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
dsunit = dswidth = 0;
else {
fprintf(stderr,
- _("%s: Stripe unit(%d) or stripe width(%d) is "
- "not a multiple of the block size(%d)\n"),
+ _("%s: Stripe unit(%"PRIu64") or stripe width(%"PRIu64") is "
+ "not a multiple of the block size(%"PRIu64")\n"),
progname, BBTOB(dsunit), BBTOB(dswidth),
blocksize);
exit(1);
@@ -2626,7 +2626,7 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
/* Warn only if specified on commandline */
if (lsuflag || lsunitflag) {
fprintf(stderr,
- _("log stripe unit (%d bytes) is too large (maximum is 256KiB)\n"),
+ _("log stripe unit (%"PRIu64" bytes) is too large (maximum is 256KiB)\n"),
(lsunit * blocksize));
fprintf(stderr,
_("log stripe unit adjusted to 32KiB\n"));
@@ -2771,14 +2771,14 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
if (!qflag || Nflag) {
printf(_(
- "meta-data=%-22s isize=%-6d agcount=%lld, agsize=%lld blks\n"
- " =%-22s sectsz=%-5u attr=%u, projid32bit=%u\n"
+ "meta-data=%-22s isize=%-6lu agcount=%lld, agsize=%lld blks\n"
+ " =%-22s sectsz=%-5lu attr=%u, projid32bit=%u\n"
" =%-22s crc=%-8u finobt=%u, sparse=%u, rmapbt=%u, reflink=%u\n"
- "data =%-22s bsize=%-6u blocks=%llu, imaxpct=%u\n"
- " =%-22s sunit=%-6u swidth=%u blks\n"
- "naming =version %-14u bsize=%-6u ascii-ci=%d ftype=%d\n"
+ "data =%-22s bsize=%-6lu blocks=%llu, imaxpct=%"PRIu64"\n"
+ " =%-22s sunit=%-6lu swidth=%"PRIu64" blks\n"
+ "naming =version %-14u bsize=%-6lu ascii-ci=%d ftype=%d\n"
"log =%-22s bsize=%-6d blocks=%lld, version=%d\n"
- " =%-22s sectsz=%-5u sunit=%d blks, lazy-count=%d\n"
+ " =%-22s sectsz=%-5lu sunit=%"PRIu64" blks, lazy-count=%d\n"
"realtime =%-22s extsz=%-6d blocks=%lld, rtextents=%lld\n"),
dfile, isize, (long long)agcount, (long long)agsize,
"", sectorsize, sb_feat.attr_version,
@@ -3431,17 +3431,17 @@ unknown(
usage();
}
-long long
+uint64_t
cvtnum(
unsigned int blksize,
unsigned int sectsize,
const char *s)
{
- long long i;
+ uint64_t i;
char *sp;
int c;
- i = strtoll(s, &sp, 0);
+ i = strtoull(s, &sp, 0);
if (i == 0 && sp == s)
return -1LL;
if (*sp == '\0')
--
2.1.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v3] mkfs: unify numeric types of main variables in main()
2017-04-20 13:58 ` [PATCH 1/2 v3] " Jan Tulak
@ 2017-04-25 1:37 ` Luis R. Rodriguez
2017-04-25 12:07 ` Jan Tulak
0 siblings, 1 reply; 17+ messages in thread
From: Luis R. Rodriguez @ 2017-04-25 1:37 UTC (permalink / raw)
To: Jan Tulak; +Cc: linux-xfs
On Thu, Apr 20, 2017 at 03:58:39PM +0200, Jan Tulak wrote:
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 7a5c49f..40a32be 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -3431,17 +3431,17 @@ unknown(
> usage();
> }
>
> -long long
> +uint64_t
> cvtnum(
> unsigned int blksize,
> unsigned int sectsize,
> const char *s)
> {
> - long long i;
> + uint64_t i;
> char *sp;
> int c;
>
> - i = strtoll(s, &sp, 0);
> + i = strtoull(s, &sp, 0);
> if (i == 0 && sp == s)
> return -1LL;
> if (*sp == '\0')
I'm afraid this will not cut it, you see before we accepted negative values
and used it as mechanism to catch errors, see the above return -1LL; with this
change we'd only catch an error in parsing if the subopt's maxval happens to be
smaller than -1LL which in turn will be returned as a positive value.
Two more issues I spotted:
a) the else condition on getnum() to use for when !sp->convert was left in your
patch with strtoll() and I think you meant to convert that as well to
strtoull()?
b) I noted the existing cvtnum() code does not check for wrap arounds in its
extra conversions.
At first glance it may seem the best option is to modify the prototype of
cvtnum() to return int to interpret errors, and have it set the uint64_t
through a parameter pointer. The wrap around issue is orthogonal and would
be an old issue, but such a change would help treat these as follows but
as I will explain below this is perhaps not best for now.
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index ef40c9a36e40..5995245e471d 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1410,6 +1410,7 @@ getnum(
{
struct subopt_param *sp = &opts->subopt_params[index];
uint64_t c;
+ int ret;
check_opt(opts, index, false);
set_conf_raw(opts->index, index, str);
@@ -1434,13 +1435,16 @@ getnum(
* convert it ourselves to guarantee there is no trailing garbage in the
* number.
*/
- if (sp->convert)
- c = cvtnum(get_conf_val(OPT_B, B_SIZE),
- get_conf_val(OPT_D, D_SECTSIZE), str);
- else {
+ if (sp->convert) {
+ ret = cvtnum(get_conf_val(OPT_B, B_SIZE),
+ get_conf_val(OPT_D, D_SECTSIZE), str, &c);
+ if (ret)
+ illegal_option(str, opts, index,
+ _("Parse error, ret: %d", ret));
+ } else {
char *str_end;
- c = strtoll(str, &str_end, 0);
+ c = strtoull(str, &str_end, 0);
if (c == 0 && str_end == str)
illegal_option(str, opts, index, NULL);
if (*str_end != '\0')
@@ -3814,24 +3818,25 @@ unknown(
usage();
}
-uint64_t
+int
cvtnum(
unsigned int blksize,
unsigned int sectsize,
- const char *s)
+ const char *s,
+ uint64_t *val)
{
uint64_t i;
char *sp;
int c;
+ uint64_t orig_val;
i = strtoull(s, &sp, 0);
if (i == 0 && sp == s)
- return -1LL;
+ return -EINVAL;
if (*sp == '\0')
- return i;
-
+ return -EINVAL;
if (sp[1] != '\0')
- return -1LL;
+ return -EINVAL;
if (*sp == 'b') {
if (!blksize) {
@@ -3839,7 +3844,10 @@ cvtnum(
_("Blocksize must be provided prior to using 'b' suffix.\n"));
usage();
} else {
- return i * blksize;
+ *val = i * blksize;
+ if (*val < i || *val < blksize)
+ return -ERANGE;
+ return 0;
}
}
if (*sp == 's') {
@@ -3848,11 +3856,15 @@ _("Blocksize must be provided prior to using 'b' suffix.\n"));
_("Sectorsize must be specified prior to using 's' suffix.\n"));
usage();
} else {
- return i * sectsize;
+ *val = i * sectsize;
+ if (*val < i || *val < sectsize)
+ return -ERANGE;
+ return 0;
}
}
c = tolower(*sp);
+ orig_val = i;
switch (c) {
case 'e':
i *= 1024LL;
@@ -3870,11 +3882,14 @@ _("Sectorsize must be specified prior to using 's' suffix.\n"));
i *= 1024LL;
/* fall through */
case 'k':
- return i * 1024LL;
+ *val = i * 1024LL;
+ if (*val < orig_val)
+ return -ERANGE;
+ return 0;
default:
break;
}
- return -1LL;
+ return -EINVAL;
}
static void __attribute__((noreturn))
The issue with this of course is everyone and their mom calls cvtnum() and the
amount of collateral for such type of change is significant for your patch series.
One option may just be to bail on error within cvtnum() with a usage() call on
error, as a temporary compromise, this way we only chug on *iff* the value was
accepted and proper, and non-wrap-around, up to you.
Luis
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] mkfs: remove long long type casts
2017-04-19 15:30 ` [PATCH 2/2] mkfs: remove long long type casts Jan Tulak
2017-04-20 8:33 ` [PATCH 2/2 v2] " Jan Tulak
@ 2017-04-25 1:40 ` Luis R. Rodriguez
1 sibling, 0 replies; 17+ messages in thread
From: Luis R. Rodriguez @ 2017-04-25 1:40 UTC (permalink / raw)
To: Jan Tulak; +Cc: linux-xfs
On Wed, Apr 19, 2017 at 05:30:25PM +0200, Jan Tulak wrote:
> We have uint64, why cast it to long long for prints? Just print it as
> it is.
>
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
Reviewed-by: Luis R. Rodriguez <mcgrof@kernel.org>
Luis
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v3] mkfs: unify numeric types of main variables in main()
2017-04-25 1:37 ` Luis R. Rodriguez
@ 2017-04-25 12:07 ` Jan Tulak
2017-04-26 1:57 ` Luis R. Rodriguez
0 siblings, 1 reply; 17+ messages in thread
From: Jan Tulak @ 2017-04-25 12:07 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linux-xfs
On Tue, Apr 25, 2017 at 3:37 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Thu, Apr 20, 2017 at 03:58:39PM +0200, Jan Tulak wrote:
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index 7a5c49f..40a32be 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -3431,17 +3431,17 @@ unknown(
>> usage();
>> }
>>
>> -long long
>> +uint64_t
>> cvtnum(
>> unsigned int blksize,
>> unsigned int sectsize,
>> const char *s)
>> {
>> - long long i;
>> + uint64_t i;
>> char *sp;
>> int c;
>>
>> - i = strtoll(s, &sp, 0);
>> + i = strtoull(s, &sp, 0);
>> if (i == 0 && sp == s)
>> return -1LL;
>> if (*sp == '\0')
>
> I'm afraid this will not cut it, you see before we accepted negative values
> and used it as mechanism to catch errors, see the above return -1LL; with this
> change we'd only catch an error in parsing if the subopt's maxval happens to be
> smaller than -1LL which in turn will be returned as a positive value.
>
> Two more issues I spotted:
>
> a) the else condition on getnum() to use for when !sp->convert was left in your
> patch with strtoll() and I think you meant to convert that as well to
> strtoull()?
>
> b) I noted the existing cvtnum() code does not check for wrap arounds in its
> extra conversions.
>
> At first glance it may seem the best option is to modify the prototype of
> cvtnum() to return int to interpret errors, and have it set the uint64_t
> through a parameter pointer. The wrap around issue is orthogonal and would
> be an old issue, but such a change would help treat these as follows but
> as I will explain below this is perhaps not best for now.
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index ef40c9a36e40..5995245e471d 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1410,6 +1410,7 @@ getnum(
> {
> struct subopt_param *sp = &opts->subopt_params[index];
> uint64_t c;
> + int ret;
>
> check_opt(opts, index, false);
> set_conf_raw(opts->index, index, str);
> @@ -1434,13 +1435,16 @@ getnum(
> * convert it ourselves to guarantee there is no trailing garbage in the
> * number.
> */
> - if (sp->convert)
> - c = cvtnum(get_conf_val(OPT_B, B_SIZE),
> - get_conf_val(OPT_D, D_SECTSIZE), str);
> - else {
> + if (sp->convert) {
> + ret = cvtnum(get_conf_val(OPT_B, B_SIZE),
> + get_conf_val(OPT_D, D_SECTSIZE), str, &c);
> + if (ret)
> + illegal_option(str, opts, index,
> + _("Parse error, ret: %d", ret));
This printing won't work - we would have to use a sprintf, with all
the prealocation for the new string and so on. Or as I'm modifying it,
replace the if (ret) with a switch(ret) a print directly EINVAL/ERANGE
as part of the string.
> + } else {
> char *str_end;
>
> - c = strtoll(str, &str_end, 0);
> + c = strtoull(str, &str_end, 0);
> if (c == 0 && str_end == str)
> illegal_option(str, opts, index, NULL);
> if (*str_end != '\0')
> @@ -3814,24 +3818,25 @@ unknown(
> usage();
> }
>
> -uint64_t
> +int
> cvtnum(
> unsigned int blksize,
> unsigned int sectsize,
> - const char *s)
> + const char *s,
> + uint64_t *val)
> {
> uint64_t i;
> char *sp;
> int c;
> + uint64_t orig_val;
>
> i = strtoull(s, &sp, 0);
> if (i == 0 && sp == s)
> - return -1LL;
> + return -EINVAL;
> if (*sp == '\0')
> - return i;
> -
> + return -EINVAL;
This has to be *val = i; return 0;
Otherwise, we would report numbers without any suffix as an error. ;-)
> if (sp[1] != '\0')
> - return -1LL;
> + return -EINVAL;
>
> if (*sp == 'b') {
> if (!blksize) {
> @@ -3839,7 +3844,10 @@ cvtnum(
> _("Blocksize must be provided prior to using 'b' suffix.\n"));
> usage();
> } else {
> - return i * blksize;
> + *val = i * blksize;
> + if (*val < i || *val < blksize)
> + return -ERANGE;
> + return 0;
> }
> }
> if (*sp == 's') {
> @@ -3848,11 +3856,15 @@ _("Blocksize must be provided prior to using 'b' suffix.\n"));
> _("Sectorsize must be specified prior to using 's' suffix.\n"));
> usage();
> } else {
> - return i * sectsize;
> + *val = i * sectsize;
> + if (*val < i || *val < sectsize)
> + return -ERANGE;
> + return 0;
> }
> }
>
> c = tolower(*sp);
> + orig_val = i;
> switch (c) {
> case 'e':
> i *= 1024LL;
> @@ -3870,11 +3882,14 @@ _("Sectorsize must be specified prior to using 's' suffix.\n"));
> i *= 1024LL;
> /* fall through */
> case 'k':
> - return i * 1024LL;
> + *val = i * 1024LL;
> + if (*val < orig_val)
> + return -ERANGE;
> + return 0;
> default:
> break;
> }
> - return -1LL;
> + return -EINVAL;
> }
>
> static void __attribute__((noreturn))
>
> The issue with this of course is everyone and their mom calls cvtnum() and the
> amount of collateral for such type of change is significant for your patch series.
> One option may just be to bail on error within cvtnum() with a usage() call on
> error, as a temporary compromise, this way we only chug on *iff* the value was
> accepted and proper, and non-wrap-around, up to you.
>
> Luis
Ehh, it is not really an issue.. cvtnum is called only on two places
in the whole xfsprogs. Changing mkfs/proto.c is just few lines added
to this patch and the changes in xfs_mkfs.c do cause few conflicts,
but it is nothing terrific, I rebased all my further changes in about
three minutes. I pushed it into the git tree, check it now...
And thanks for this patch for the patch. :-)
Jan
--
Jan Tulak
jtulak@redhat.com / jan@tulak.me
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v3] mkfs: unify numeric types of main variables in main()
2017-04-25 12:07 ` Jan Tulak
@ 2017-04-26 1:57 ` Luis R. Rodriguez
2017-04-26 8:03 ` Jan Tulak
0 siblings, 1 reply; 17+ messages in thread
From: Luis R. Rodriguez @ 2017-04-26 1:57 UTC (permalink / raw)
To: Jan Tulak; +Cc: Luis R. Rodriguez, linux-xfs
On Tue, Apr 25, 2017 at 02:07:02PM +0200, Jan Tulak wrote:
> Ehh, it is not really an issue.. cvtnum is called only on two places
> in the whole xfsprogs.
Huh, I count 46, and spread all over the place:
mcgrof@ergon ~/devel/xfsprogs-dev (git::libiniconfig-conf)$ git grep " cvtnum("| awk '{print $1}'| sort | uniq
include/input.h:extern
include/xfs_multidisk.h:extern
io/fadvise.c:
io/madvise.c:
io/mincore.c:
io/mmap.c:
io/pread.c:
io/prealloc.c:
io/pwrite.c:
io/readdir.c:
io/reflink.c:
io/resblks.c:
io/seek.c:
io/sendfile.c:
io/sync_file_range.c:
io/truncate.c:
mkfs/proto.c:
mkfs/xfs_mkfs.c:
quota/edit.c:
So 19 files.
> Changing mkfs/proto.c is just few lines added
> to this patch and the changes in xfs_mkfs.c do cause few conflicts,
> but it is nothing terrific, I rebased all my further changes in about
> three minutes. I pushed it into the git tree, check it now...
Will do...
> And thanks for this patch for the patch. :-)
My pleasure, on second thought if the wrap around change can be a separate
atomic change that might be worth it.
Luis
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v2] mkfs: unify numeric types of main variables in main()
2017-04-20 8:33 ` [PATCH 1/2 v2] " Jan Tulak
2017-04-20 13:28 ` Luis R. Rodriguez
@ 2017-04-26 3:58 ` Eric Sandeen
2017-04-26 7:58 ` Jan Tulak
1 sibling, 1 reply; 17+ messages in thread
From: Eric Sandeen @ 2017-04-26 3:58 UTC (permalink / raw)
To: Jan Tulak, linux-xfs
On 4/20/17 3:33 AM, Jan Tulak wrote:
> In the past, when mkfs was first written, it used atoi and
> similar calls, so the variables were ints. However, the situation moved
> since then and in course of the time, mkfs began to use other types too.
>
> Clean and unify it. We don't need negative values anywhere in the code and
> some numbers has to be 64bit. Thus, uint64_t is the best candidate as the target
> type for numeric values. The existing uses of __uint64_t in mkfs change to the
> standard uint64_t type. For flag values let's use boolean.
>
> This patch changes variables declared at the beginning of main() +
> block/sectorsize, making only minimal changes. The following patch
> cleans some now-unnecessary type casts.
>
> It would be nice to change types in some of the structures too, but
> this might lead to changes outside of mkfs, so I'm skipping them for
> this moment to keep it simple.
This introduces quite a few new warnings on i386, first of all.
(they persist with patch 2/2), so please sort that out.
Also, almost all of the existing use of PRIu64 in the codebase
puts spaces around it, i.e.
("The value is %" PRIu64 "\n")
not
("The value is %"PRIu64"\n")
so I'd prefer that you follow that convention whenever you
need to use it.
On a respin, it would be nice to separate the boolean changes
into a separate patch from the 64-bit changes, to produce
shorter individual patches to aid review. In fact, splitting out
another patch to first convert existing __uint64_t to uint64_t would
make it even nicer. Small, self-contained, functional patches.
Change one thing at a time, especially for long patches.
(Splitting out the bools would make it more obvious that "discard" and
"lalign" really should be converted to bools below, not uint64_t
as they are now; there might be others.)
However, as Dave said earlier:
> we need to work
> towards removing all the variables, not try to make them pretty....
and indeed, your later patches do remove a lot of the variables that
you're converting here, i.e.:
[PATCH 08/12] mkfs: replace variables with opts table: -b,d,s options
does:
-uint64_t blocksize;
-uint64_t sectorsize;
...
- uint64_t agcount;
- uint64_t agsize;
- uint64_t blocklog;
...
- uint64_t dbytes;
- uint64_t dsu;
- uint64_t dsw;
- uint64_t dsunit;
- uint64_t dswidth;
...
- uint64_t sectorlog;
and [PATCH 09/12] mkfs: replace variables with opts table: -i options
does:
- uint64_t imaxpct;
- uint64_t inodelog;
- uint64_t inopblock;
- uint64_t isize;
etc ...
so is there any real advantage to converting all of these, only to remove
most of them a few patches later? (If there is, just help me understand
why...)
Thanks,
-Eric
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> ---
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v2] mkfs: unify numeric types of main variables in main()
2017-04-26 3:58 ` Eric Sandeen
@ 2017-04-26 7:58 ` Jan Tulak
2017-05-09 15:49 ` Eric Sandeen
0 siblings, 1 reply; 17+ messages in thread
From: Jan Tulak @ 2017-04-26 7:58 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
On Wed, Apr 26, 2017 at 5:58 AM, Eric Sandeen <sandeen@sandeen.net> wrote:
> On 4/20/17 3:33 AM, Jan Tulak wrote:
>> In the past, when mkfs was first written, it used atoi and
>> similar calls, so the variables were ints. However, the situation moved
>> since then and in course of the time, mkfs began to use other types too.
>>
>> Clean and unify it. We don't need negative values anywhere in the code and
>> some numbers has to be 64bit. Thus, uint64_t is the best candidate as the target
>> type for numeric values. The existing uses of __uint64_t in mkfs change to the
>> standard uint64_t type. For flag values let's use boolean.
>>
>> This patch changes variables declared at the beginning of main() +
>> block/sectorsize, making only minimal changes. The following patch
>> cleans some now-unnecessary type casts.
>>
>> It would be nice to change types in some of the structures too, but
>> this might lead to changes outside of mkfs, so I'm skipping them for
>> this moment to keep it simple.
>
> This introduces quite a few new warnings on i386, first of all.
> (they persist with patch 2/2), so please sort that out.
ok
>
> Also, almost all of the existing use of PRIu64 in the codebase
> puts spaces around it, i.e.
> ("The value is %" PRIu64 "\n")
> not
> ("The value is %"PRIu64"\n")
>
> so I'd prefer that you follow that convention whenever you
> need to use it.
>
> On a respin, it would be nice to separate the boolean changes
> into a separate patch from the 64-bit changes, to produce
> shorter individual patches to aid review. In fact, splitting out
> another patch to first convert existing __uint64_t to uint64_t would
> make it even nicer. Small, self-contained, functional patches.
> Change one thing at a time, especially for long patches.
>
> (Splitting out the bools would make it more obvious that "discard" and
> "lalign" really should be converted to bools below, not uint64_t
> as they are now; there might be others.)
>
Good idea.
> However, as Dave said earlier:
>
>> we need to work
>> towards removing all the variables, not try to make them pretty....
>
> and indeed, your later patches do remove a lot of the variables that
> you're converting here, i.e.:
>
> [PATCH 08/12] mkfs: replace variables with opts table: -b,d,s options
>
> does:
>
> -uint64_t blocksize;
> -uint64_t sectorsize;
> ...
> - uint64_t agcount;
> - uint64_t agsize;
> - uint64_t blocklog;
> ...
> - uint64_t dbytes;
> - uint64_t dsu;
> - uint64_t dsw;
> - uint64_t dsunit;
> - uint64_t dswidth;
> ...
> - uint64_t sectorlog;
>
> and [PATCH 09/12] mkfs: replace variables with opts table: -i options
> does:
>
> - uint64_t imaxpct;
> - uint64_t inodelog;
> - uint64_t inopblock;
> - uint64_t isize;
>
> etc ...
>
> so is there any real advantage to converting all of these, only to remove
> most of them a few patches later? (If there is, just help me understand
> why...)
>
I wanted to avoid type changes in patches that replace variables in main
with opts use. Without this, I need to have the opts members in long
long, but I still get some changes int -> long long and even one or two
uint -> long long. So I wanted to have all the replaced variables already in a
single type, without the need to change format strings and knowing
that the type change is not causing any side effect.
But I think this shouldn't be a big issue (it doesn't seem that type
changes broke anything), so I'm rebasing the set without these uint
changes and the type transition can happen later on.
Cheers,
Jan
> Thanks,
> -Eric
>
>> Signed-off-by: Jan Tulak <jtulak@redhat.com>
>> ---
--
Jan Tulak
jtulak@redhat.com / jan@tulak.me
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v3] mkfs: unify numeric types of main variables in main()
2017-04-26 1:57 ` Luis R. Rodriguez
@ 2017-04-26 8:03 ` Jan Tulak
0 siblings, 0 replies; 17+ messages in thread
From: Jan Tulak @ 2017-04-26 8:03 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linux-xfs
On Wed, Apr 26, 2017 at 3:57 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Tue, Apr 25, 2017 at 02:07:02PM +0200, Jan Tulak wrote:
>> Ehh, it is not really an issue.. cvtnum is called only on two places
>> in the whole xfsprogs.
>
> Huh, I count 46, and spread all over the place:
>
> mcgrof@ergon ~/devel/xfsprogs-dev (git::libiniconfig-conf)$ git grep " cvtnum("| awk '{print $1}'| sort | uniq
> include/input.h:extern
> include/xfs_multidisk.h:extern
> io/fadvise.c:
> io/madvise.c:
> io/mincore.c:
> io/mmap.c:
> io/pread.c:
> io/prealloc.c:
> io/pwrite.c:
> io/readdir.c:
> io/reflink.c:
> io/resblks.c:
> io/seek.c:
> io/sendfile.c:
> io/sync_file_range.c:
> io/truncate.c:
> mkfs/proto.c:
> mkfs/xfs_mkfs.c:
> quota/edit.c:
>
> So 19 files.
They use a different prototype/implementation. If you change the
prototype of cvtnum in include/xfs_multidisk.h and mkfs
implementation, only one call in mkfs's getnum and in mkfs/proto.c's
getnum complains. There are three implementations of cvtnum in
xfsprogs.
>
>> Changing mkfs/proto.c is just few lines added
>> to this patch and the changes in xfs_mkfs.c do cause few conflicts,
>> but it is nothing terrific, I rebased all my further changes in about
>> three minutes. I pushed it into the git tree, check it now...
>
> Will do...
>
>> And thanks for this patch for the patch. :-)
>
> My pleasure, on second thought if the wrap around change can be a separate
> atomic change that might be worth it.
>
Yeah, I realised that too... I will split it if I don't forget.
Cheers,
Jan
> Luis
--
Jan Tulak
jtulak@redhat.com / jan@tulak.me
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2 v2] mkfs: unify numeric types of main variables in main()
2017-04-26 7:58 ` Jan Tulak
@ 2017-05-09 15:49 ` Eric Sandeen
0 siblings, 0 replies; 17+ messages in thread
From: Eric Sandeen @ 2017-05-09 15:49 UTC (permalink / raw)
To: Jan Tulak; +Cc: linux-xfs
On 4/26/17 2:58 AM, Jan Tulak wrote:
>> so is there any real advantage to converting all of these, only to remove
>> most of them a few patches later? (If there is, just help me understand
>> why...)
>>
> I wanted to avoid type changes in patches that replace variables in main
> with opts use. Without this, I need to have the opts members in long
> long, but I still get some changes int -> long long and even one or two
> uint -> long long. So I wanted to have all the replaced variables already in a
> single type, without the need to change format strings and knowing
> that the type change is not causing any side effect.
OK, fair enough.
> But I think this shouldn't be a big issue (it doesn't seem that type
> changes broke anything), so I'm rebasing the set without these uint
> changes and the type transition can happen later on.
Your argument above was reasonable, if you want to change them all for
consistency before re-organization, that's ok too. Either way.
-Eric
> Cheers,
> Jan
>
>> Thanks,
>> -Eric
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-05-09 15:50 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-19 15:30 [PATCH 1/2] mkfs: unify numeric types of main variables in main() Jan Tulak
2017-04-19 15:30 ` [PATCH 2/2] mkfs: remove long long type casts Jan Tulak
2017-04-20 8:33 ` [PATCH 2/2 v2] " Jan Tulak
2017-04-25 1:40 ` [PATCH 2/2] " Luis R. Rodriguez
2017-04-20 0:09 ` [PATCH 1/2] mkfs: unify numeric types of main variables in main() Dave Chinner
2017-04-20 8:06 ` Jan Tulak
2017-04-20 8:33 ` [PATCH 1/2 v2] " Jan Tulak
2017-04-20 13:28 ` Luis R. Rodriguez
2017-04-20 13:57 ` Jan Tulak
2017-04-26 3:58 ` Eric Sandeen
2017-04-26 7:58 ` Jan Tulak
2017-05-09 15:49 ` Eric Sandeen
2017-04-20 13:58 ` [PATCH 1/2 v3] " Jan Tulak
2017-04-25 1:37 ` Luis R. Rodriguez
2017-04-25 12:07 ` Jan Tulak
2017-04-26 1:57 ` Luis R. Rodriguez
2017-04-26 8:03 ` Jan Tulak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).