| Message ID | 20171113144527.1034-1-chandan@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 13, 2017 at 4:45 PM, Chandan Rajendra <chandan@linux.vnet.ibm.com> wrote: > An overlayfs filesystem instance with one lowerdir filesystem and with > "xino" mount option enabled can have the layer index encoded in the 63rd > bit of the inode number. A signed 64 bit integer won't suffice to store > this inode number. Hence this commit uses strtoul() to convert the inode > number in string form to unsigned integer form. > > Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com> Looks good, especially since I had to fix the same problem myself ;) https://github.com/amir73il/xfstests/commits/overlayfs-devel My patch also changes: int type = -1; /* -1 means all types */ - uint64_t ino = 0; + unsigned long ino = 0; int ret = 1; But I am not sure that is the right thing to do here or what difference it makes Amir. > --- > src/t_dir_type.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/t_dir_type.c b/src/t_dir_type.c > index 76aaa9b..7bba304 100644 > --- a/src/t_dir_type.c > +++ b/src/t_dir_type.c > @@ -85,7 +85,7 @@ main(int argc, char *argv[]) > break; > /* no match ends up with type = -1 */ > if (type < 0) > - ino = atoll(argv[2]); > + ino = strtoul(argv[2], NULL, 10); > } > > for ( ; ; ) { > -- > 2.9.5 > -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 13, 2017 at 04:57:08PM +0200, Amir Goldstein wrote: > On Mon, Nov 13, 2017 at 4:45 PM, Chandan Rajendra > <chandan@linux.vnet.ibm.com> wrote: > > An overlayfs filesystem instance with one lowerdir filesystem and with > > "xino" mount option enabled can have the layer index encoded in the 63rd > > bit of the inode number. A signed 64 bit integer won't suffice to store > > this inode number. Hence this commit uses strtoul() to convert the inode > > number in string form to unsigned integer form. > > > > Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com> > > Looks good, especially since I had to fix the same problem myself ;) > https://github.com/amir73il/xfstests/commits/overlayfs-devel > > My patch also changes: > > int type = -1; /* -1 means all types */ > - uint64_t ino = 0; > + unsigned long ino = 0; > int ret = 1; > > But I am not sure that is the right thing to do here or what difference it makes I think that strtoul() returns unsigned long, which could be 32bit, and uint64_t is guaranteed to be 64bit size, so I tend to change the ino definition too. But I guess that doesn't matter that much :) Thanks, Eryu > > Amir. > > > --- > > src/t_dir_type.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/t_dir_type.c b/src/t_dir_type.c > > index 76aaa9b..7bba304 100644 > > --- a/src/t_dir_type.c > > +++ b/src/t_dir_type.c > > @@ -85,7 +85,7 @@ main(int argc, char *argv[]) > > break; > > /* no match ends up with type = -1 */ > > if (type < 0) > > - ino = atoll(argv[2]); > > + ino = strtoul(argv[2], NULL, 10); > > } > > > > for ( ; ; ) { > > -- > > 2.9.5 > > -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 14, 2017 at 1:13 PM, Eryu Guan <eguan@redhat.com> wrote: > On Mon, Nov 13, 2017 at 04:57:08PM +0200, Amir Goldstein wrote: >> On Mon, Nov 13, 2017 at 4:45 PM, Chandan Rajendra >> <chandan@linux.vnet.ibm.com> wrote: >> > An overlayfs filesystem instance with one lowerdir filesystem and with >> > "xino" mount option enabled can have the layer index encoded in the 63rd >> > bit of the inode number. A signed 64 bit integer won't suffice to store >> > this inode number. Hence this commit uses strtoul() to convert the inode >> > number in string form to unsigned integer form. >> > >> > Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com> >> >> Looks good, especially since I had to fix the same problem myself ;) >> https://github.com/amir73il/xfstests/commits/overlayfs-devel >> >> My patch also changes: >> >> int type = -1; /* -1 means all types */ >> - uint64_t ino = 0; >> + unsigned long ino = 0; >> int ret = 1; >> >> But I am not sure that is the right thing to do here or what difference it makes > > I think that strtoul() returns unsigned long, which could be 32bit, and > uint64_t is guaranteed to be 64bit size, so I tend to change the ino > definition too. But I guess that doesn't matter that much :) > The thing is that 'ino' is later compared with 'd->d_ino', which is uint64_t, so on 32bit CPU, the conversion will happen either in assignment from strtoul() or in comparison later. I guess it doesn't matter much, so I prefer Chandan's version that leaves ino as uint64_t. Amir. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 14, 2017 at 01:23:18PM +0200, Amir Goldstein wrote: > On Tue, Nov 14, 2017 at 1:13 PM, Eryu Guan <eguan@redhat.com> wrote: > > On Mon, Nov 13, 2017 at 04:57:08PM +0200, Amir Goldstein wrote: > >> On Mon, Nov 13, 2017 at 4:45 PM, Chandan Rajendra > >> <chandan@linux.vnet.ibm.com> wrote: > >> > An overlayfs filesystem instance with one lowerdir filesystem and with > >> > "xino" mount option enabled can have the layer index encoded in the 63rd > >> > bit of the inode number. A signed 64 bit integer won't suffice to store > >> > this inode number. Hence this commit uses strtoul() to convert the inode > >> > number in string form to unsigned integer form. > >> > > >> > Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com> > >> > >> Looks good, especially since I had to fix the same problem myself ;) > >> https://github.com/amir73il/xfstests/commits/overlayfs-devel > >> > >> My patch also changes: > >> > >> int type = -1; /* -1 means all types */ > >> - uint64_t ino = 0; > >> + unsigned long ino = 0; > >> int ret = 1; > >> > >> But I am not sure that is the right thing to do here or what difference it makes > > > > I think that strtoul() returns unsigned long, which could be 32bit, and > > uint64_t is guaranteed to be 64bit size, so I tend to change the ino > > definition too. But I guess that doesn't matter that much :) > > > > The thing is that 'ino' is later compared with 'd->d_ino', which is uint64_t, > so on 32bit CPU, the conversion will happen either in assignment from > strtoul() or in comparison later. I guess it doesn't matter much, so I prefer > Chandan's version that leaves ino as uint64_t. Yeah, I noticed that too. I'll take it as-is, thanks! Eryu -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/src/t_dir_type.c b/src/t_dir_type.c index 76aaa9b..7bba304 100644 --- a/src/t_dir_type.c +++ b/src/t_dir_type.c @@ -85,7 +85,7 @@ main(int argc, char *argv[]) break; /* no match ends up with type = -1 */ if (type < 0) - ino = atoll(argv[2]); + ino = strtoul(argv[2], NULL, 10); } for ( ; ; ) {
An overlayfs filesystem instance with one lowerdir filesystem and with "xino" mount option enabled can have the layer index encoded in the 63rd bit of the inode number. A signed 64 bit integer won't suffice to store this inode number. Hence this commit uses strtoul() to convert the inode number in string form to unsigned integer form. Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com> --- src/t_dir_type.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)