threads / patch / 24063

patchread_cache: cast types from stat to ce_cache

Subject: [PATCH] read_cache: cast types from stat to ce_cache

## tl;dr

5 messages between Jun 10, 2010 and Jun 11, 2010. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Michael J Gruber· Jun 10, 2010, 09:15 UTC · lore

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 <git@drmicha.warpmail.net>
---
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(-)
Show changes to read-cache.c +5 −5
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· Jun 10, 2010, 15:54 UTC · re: Michael J Gruber · lore

Re: [PATCH] read_cache: cast types from stat to ce_cache

Michael J Gruber <git@drmicha.warpmail.net> writes:
Show 10 quoted lines
> -	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· Jun 11, 2010, 13:32 UTC · re: Junio C Hamano · lore

Re: [PATCH] read_cache: cast types from stat to ce_cache

Junio C Hamano venit, vidit, dixit 10.06.2010 17:54:
Show 15 quoted lines
> Michael J Gruber <git@drmicha.warpmail.net> 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· Jun 11, 2010, 14:15 UTC · re: Michael J Gruber · lore

Re: [PATCH] read_cache: cast types from stat to ce_cache

On Fri, Jun 11, 2010 at 3:32 PM, Michael J Gruber <git@drmicha.warpmail.net> wrote:

Show 20 quoted lines
> Junio C Hamano venit, vidit, dixit 10.06.2010 17:54:
>> Michael J Gruber <git@drmicha.warpmail.net> 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· Jun 11, 2010, 14:59 UTC · re: Erik Faye-Lund · lore

Re: [PATCH] read_cache: cast types from stat to ce_cache

Erik Faye-Lund venit, vidit, dixit 11.06.2010 16:15:
Show 37 quoted lines
> On Fri, Jun 11, 2010 at 3:32 PM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Junio C Hamano venit, vidit, dixit 10.06.2010 17:54:
>>> Michael J Gruber <git@drmicha.warpmail.net> 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

← back to recent threads