# [PATCH] read_cache: cast types from stat to ce_cache 5 messages from 2010-06-10 to 2010-06-11. Participants: Michael J Gruber, Junio C Hamano, Erik Faye-Lund. Thread: https://gitlist.dev/t/24063 ## Michael J Gruber, 2010-06-10 09:15 Subject: [PATCH] read_cache: cast types from stat to ce_cache Message-ID: URL: https://gitlist.dev/e/a06cc84698f174d914350d6fd6e6b4739d22aa45.1276161176.git.git%40drmicha.warpmail.net ``` Cast the system dependent entries of struct stat to unsigned integers for struct ce_cache just like in other places where we do this already, to protect against any type conversion issues. Signed-off-by: Michael J Gruber --- This does not fix the "filesystem boundary issue" I reported, but I spotted this when digging through read-cache.c. My device ids would have issues when int = short. read-cache.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/read-cache.c b/read-cache.c index e381ea5..aa9b847 100644 --- a/read-cache.c +++ b/read-cache.c @@ -72,11 +72,11 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st) ce->ce_mtime.sec = (unsigned int)st->st_mtime; ce->ce_ctime.nsec = ST_CTIME_NSEC(*st); ce->ce_mtime.nsec = ST_MTIME_NSEC(*st); - ce->ce_dev = st->st_dev; - ce->ce_ino = st->st_ino; - ce->ce_uid = st->st_uid; - ce->ce_gid = st->st_gid; - ce->ce_size = st->st_size; + ce->ce_dev = (unsigned int)st->st_dev; + ce->ce_ino = (unsigned int)st->st_ino; + ce->ce_uid = (unsigned int)st->st_uid; + ce->ce_gid = (unsigned int)st->st_gid; + ce->ce_size = (unsigned int)st->st_size; if (assume_unchanged) ce->ce_flags |= CE_VALID; -- 1.7.1.501.g23b46 ``` ## Junio C Hamano, 2010-06-10 15:54 Subject: Re: [PATCH] read_cache: cast types from stat to ce_cache Message-ID: <7vmxv2nar5.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vmxv2nar5.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Michael J Gruber writes: > - ce->ce_dev = st->st_dev; > - ce->ce_ino = st->st_ino; > - ce->ce_uid = st->st_uid; > - ce->ce_gid = st->st_gid; > - ce->ce_size = st->st_size; > + ce->ce_dev = (unsigned int)st->st_dev; > + ce->ce_ino = (unsigned int)st->st_ino; > + ce->ce_uid = (unsigned int)st->st_uid; > + ce->ce_gid = (unsigned int)st->st_gid; > + ce->ce_size = (unsigned int)st->st_size; I haven't had my morning coffee yet, but wouldn't the conversion be automatic by assignment anyway? ``` ## Michael J Gruber, 2010-06-11 13:32 Subject: Re: [PATCH] read_cache: cast types from stat to ce_cache Message-ID: <4C123AD4.6070606@drmicha.warpmail.net> URL: https://gitlist.dev/e/4C123AD4.6070606%40drmicha.warpmail.net In-Reply-To: <7vmxv2nar5.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano venit, vidit, dixit 10.06.2010 17:54: > Michael J Gruber writes: > >> - ce->ce_dev = st->st_dev; >> - ce->ce_ino = st->st_ino; >> - ce->ce_uid = st->st_uid; >> - ce->ce_gid = st->st_gid; >> - ce->ce_size = st->st_size; >> + ce->ce_dev = (unsigned int)st->st_dev; >> + ce->ce_ino = (unsigned int)st->st_ino; >> + ce->ce_uid = (unsigned int)st->st_uid; >> + ce->ce_gid = (unsigned int)st->st_gid; >> + ce->ce_size = (unsigned int)st->st_size; > > I haven't had my morning coffee yet, but wouldn't the conversion be > automatic by assignment anyway? Well, we do cast from dev_t etc. to unsigned int in all other places in read_cache.c. In any case, enjoy your coffee :) Michael ``` ## Erik Faye-Lund, 2010-06-11 14:15 Subject: Re: [PATCH] read_cache: cast types from stat to ce_cache Message-ID: URL: https://gitlist.dev/e/AANLkTimC9RAD0OGArk3JAIAnDHNskJRgX1nBlcrhc7vX%40mail.gmail.com In-Reply-To: <4C123AD4.6070606@drmicha.warpmail.net> ``` On Fri, Jun 11, 2010 at 3:32 PM, Michael J Gruber wrote: > Junio C Hamano venit, vidit, dixit 10.06.2010 17:54: >> Michael J Gruber writes: >> >>> -    ce->ce_dev = st->st_dev; >>> -    ce->ce_ino = st->st_ino; >>> -    ce->ce_uid = st->st_uid; >>> -    ce->ce_gid = st->st_gid; >>> -    ce->ce_size = st->st_size; >>> +    ce->ce_dev = (unsigned int)st->st_dev; >>> +    ce->ce_ino = (unsigned int)st->st_ino; >>> +    ce->ce_uid = (unsigned int)st->st_uid; >>> +    ce->ce_gid = (unsigned int)st->st_gid; >>> +    ce->ce_size = (unsigned int)st->st_size; >> >> I haven't had my morning coffee yet, but wouldn't the conversion be >> automatic by assignment anyway? > > Well, we do cast from dev_t etc. to unsigned int in all other places in > read_cache.c. > The only cases where we cast anything to unsigned in on assignment in read-cache.h is where we assign the second-part of the ce_ctime and ce_mtime members: ce->ce_ctime.sec = (unsigned int)st->st_ctime; ce->ce_mtime.sec = (unsigned int)st->st_mtime; These are indeed superfluous (as yours are) as far as I can tell. There's also some casts on comparison, and I think they also are superfluous. The current casts originate from commits fba2f38a, 5bcf109c, e1afca4f and 7a51ed66. They don't do any harm though, so I don't think it makes much sense to remove them. -- Erik "kusma" Faye-Lund ``` ## Michael J Gruber, 2010-06-11 14:59 Subject: Re: [PATCH] read_cache: cast types from stat to ce_cache Message-ID: <4C124F34.7050904@drmicha.warpmail.net> URL: https://gitlist.dev/e/4C124F34.7050904%40drmicha.warpmail.net In-Reply-To: ``` Erik Faye-Lund venit, vidit, dixit 11.06.2010 16:15: > On Fri, Jun 11, 2010 at 3:32 PM, Michael J Gruber > wrote: >> Junio C Hamano venit, vidit, dixit 10.06.2010 17:54: >>> Michael J Gruber writes: >>> >>>> - ce->ce_dev = st->st_dev; >>>> - ce->ce_ino = st->st_ino; >>>> - ce->ce_uid = st->st_uid; >>>> - ce->ce_gid = st->st_gid; >>>> - ce->ce_size = st->st_size; >>>> + ce->ce_dev = (unsigned int)st->st_dev; >>>> + ce->ce_ino = (unsigned int)st->st_ino; >>>> + ce->ce_uid = (unsigned int)st->st_uid; >>>> + ce->ce_gid = (unsigned int)st->st_gid; >>>> + ce->ce_size = (unsigned int)st->st_size; >>> >>> I haven't had my morning coffee yet, but wouldn't the conversion be >>> automatic by assignment anyway? >> >> Well, we do cast from dev_t etc. to unsigned int in all other places in >> read_cache.c. >> > > The only cases where we cast anything to unsigned in on assignment in > read-cache.h is where we assign the second-part of the ce_ctime and > ce_mtime members: > ce->ce_ctime.sec = (unsigned int)st->st_ctime; > ce->ce_mtime.sec = (unsigned int)st->st_mtime; > > These are indeed superfluous (as yours are) as far as I can tell. > There's also some casts on comparison, and I think they also are > superfluous. The current casts originate from commits fba2f38a, > 5bcf109c, e1afca4f and 7a51ed66. > > They don't do any harm though, so I don't think it makes much sense to > remove them. > OK, then we can leave them resp. leave them out until a compiler complains. I just stumbled upon this "inconsistency" when hunting down that fs boundary warning. Michael ```