From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eryu Guan Subject: Re: [PATCH 1/2] src/t_dir_type.c: Use strtoul() instead of atoll() Date: Tue, 14 Nov 2017 19:50:21 +0800 Message-ID: <20171114115021.GI17339@eguan.usersys.redhat.com> References: <20171113144527.1034-1-chandan@linux.vnet.ibm.com> <20171114111338.GG17339@eguan.usersys.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mx1.redhat.com ([209.132.183.28]:34642 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753809AbdKNLuX (ORCPT ); Tue, 14 Nov 2017 06:50:23 -0500 Content-Disposition: inline In-Reply-To: Sender: linux-unionfs-owner@vger.kernel.org List-Id: linux-unionfs@vger.kernel.org To: Amir Goldstein Cc: Chandan Rajendra , fstests , overlayfs On Tue, Nov 14, 2017 at 01:23:18PM +0200, Amir Goldstein wrote: > On Tue, Nov 14, 2017 at 1:13 PM, Eryu Guan 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 > >> 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 > >> > >> 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