{"thread":{"id":"24063","subject":"[PATCH] read_cache: cast types from stat to ce_cache","startedAt":"2010-06-10T09:15:03Z","lastAt":"2010-06-11T14:59:00Z","messageCount":5,"participants":["Michael J Gruber","Junio C Hamano","Erik Faye-Lund"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"143406","messageId":"a06cc84698f174d914350d6fd6e6b4739d22aa45.1276161176.git.git@drmicha.warpmail.net","threadId":"24063","inReplyTo":null,"subject":"[PATCH] read_cache: cast types from stat to ce_cache","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-06-10T09:15:03Z","receivedAt":"2010-06-10T09:15:03Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Cast the system dependent entries of struct stat to unsigned integers\nfor struct ce_cache just like in other places where we do this already,\nto protect against any type conversion issues.\n\nSigned-off-by: Michael J Gruber \n---\nThis does not fix the \"filesystem boundary issue\" I reported, but I spotted\nthis when digging through read-cache.c. My device ids would have issues\nwhen int = short.\n\n read-cache.c | 10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex e381ea5..aa9b847 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -72,11 +72,11 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st)\n \tce->ce_mtime.sec = (unsigned int)st->st_mtime;\n \tce->ce_ctime.nsec = ST_CTIME_NSEC(*st);\n \tce->ce_mtime.nsec = ST_MTIME_NSEC(*st);\n-\tce->ce_dev = st->st_dev;\n-\tce->ce_ino = st->st_ino;\n-\tce->ce_uid = st->st_uid;\n-\tce->ce_gid = st->st_gid;\n-\tce->ce_size = st->st_size;\n+\tce->ce_dev = (unsigned int)st->st_dev;\n+\tce->ce_ino = (unsigned int)st->st_ino;\n+\tce->ce_uid = (unsigned int)st->st_uid;\n+\tce->ce_gid = (unsigned int)st->st_gid;\n+\tce->ce_size = (unsigned int)st->st_size;\n \n \tif (assume_unchanged)\n \t\tce->ce_flags |= CE_VALID;\n-- \n1.7.1.501.g23b46\n"},{"id":"143446","messageId":"7vmxv2nar5.fsf@alter.siamese.dyndns.org","threadId":"24063","inReplyTo":"a06cc84698f174d914350d6fd6e6b4739d22aa45.1276161176.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] read_cache: cast types from stat to ce_cache","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-10T15:54:54Z","receivedAt":"2010-06-10T15:54:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> -\tce->ce_dev = st->st_dev;\n> -\tce->ce_ino = st->st_ino;\n> -\tce->ce_uid = st->st_uid;\n> -\tce->ce_gid = st->st_gid;\n> -\tce->ce_size = st->st_size;\n> +\tce->ce_dev = (unsigned int)st->st_dev;\n> +\tce->ce_ino = (unsigned int)st->st_ino;\n> +\tce->ce_uid = (unsigned int)st->st_uid;\n> +\tce->ce_gid = (unsigned int)st->st_gid;\n> +\tce->ce_size = (unsigned int)st->st_size;\n\nI haven't had my morning coffee yet, but wouldn't the conversion be\nautomatic by assignment anyway?\n"},{"id":"143507","messageId":"4C123AD4.6070606@drmicha.warpmail.net","threadId":"24063","inReplyTo":"7vmxv2nar5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] read_cache: cast types from stat to ce_cache","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-06-11T13:32:04Z","receivedAt":"2010-06-11T13:32:04Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 10.06.2010 17:54:\n> Michael J Gruber writes:\n> \n>> -\tce->ce_dev = st->st_dev;\n>> -\tce->ce_ino = st->st_ino;\n>> -\tce->ce_uid = st->st_uid;\n>> -\tce->ce_gid = st->st_gid;\n>> -\tce->ce_size = st->st_size;\n>> +\tce->ce_dev = (unsigned int)st->st_dev;\n>> +\tce->ce_ino = (unsigned int)st->st_ino;\n>> +\tce->ce_uid = (unsigned int)st->st_uid;\n>> +\tce->ce_gid = (unsigned int)st->st_gid;\n>> +\tce->ce_size = (unsigned int)st->st_size;\n> \n> I haven't had my morning coffee yet, but wouldn't the conversion be\n> automatic by assignment anyway?\n\nWell, we do cast from dev_t etc. to unsigned int in all other places in\nread_cache.c.\n\nIn any case, enjoy your coffee :)\n\nMichael\n"},{"id":"143509","messageId":"AANLkTimC9RAD0OGArk3JAIAnDHNskJRgX1nBlcrhc7vX@mail.gmail.com","threadId":"24063","inReplyTo":"4C123AD4.6070606@drmicha.warpmail.net","subject":"Re: [PATCH] read_cache: cast types from stat to ce_cache","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2010-06-11T14:15:16Z","receivedAt":"2010-06-11T14:15:16Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Fri, Jun 11, 2010 at 3:32 PM, Michael J Gruber\n wrote:\n> Junio C Hamano venit, vidit, dixit 10.06.2010 17:54:\n>> Michael J Gruber writes:\n>>\n>>> -    ce->ce_dev = st->st_dev;\n>>> -    ce->ce_ino = st->st_ino;\n>>> -    ce->ce_uid = st->st_uid;\n>>> -    ce->ce_gid = st->st_gid;\n>>> -    ce->ce_size = st->st_size;\n>>> +    ce->ce_dev = (unsigned int)st->st_dev;\n>>> +    ce->ce_ino = (unsigned int)st->st_ino;\n>>> +    ce->ce_uid = (unsigned int)st->st_uid;\n>>> +    ce->ce_gid = (unsigned int)st->st_gid;\n>>> +    ce->ce_size = (unsigned int)st->st_size;\n>>\n>> I haven't had my morning coffee yet, but wouldn't the conversion be\n>> automatic by assignment anyway?\n>\n> Well, we do cast from dev_t etc. to unsigned int in all other places in\n> read_cache.c.\n>\n\nThe only cases where we cast anything to unsigned in on assignment in\nread-cache.h is where we assign the second-part of the ce_ctime and\nce_mtime members:\n\tce->ce_ctime.sec = (unsigned int)st->st_ctime;\n\tce->ce_mtime.sec = (unsigned int)st->st_mtime;\n\nThese are indeed superfluous (as yours are) as far as I can tell.\nThere's also some casts on comparison, and I think they also are\nsuperfluous. The current casts originate from commits fba2f38a,\n5bcf109c, e1afca4f and 7a51ed66.\n\nThey don't do any harm though, so I don't think it makes much sense to\nremove them.\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"143511","messageId":"4C124F34.7050904@drmicha.warpmail.net","threadId":"24063","inReplyTo":"AANLkTimC9RAD0OGArk3JAIAnDHNskJRgX1nBlcrhc7vX@mail.gmail.com","subject":"Re: [PATCH] read_cache: cast types from stat to ce_cache","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-06-11T14:59:00Z","receivedAt":"2010-06-11T14:59:00Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Erik Faye-Lund venit, vidit, dixit 11.06.2010 16:15:\n> On Fri, Jun 11, 2010 at 3:32 PM, Michael J Gruber\n> wrote:\n>> Junio C Hamano venit, vidit, dixit 10.06.2010 17:54:\n>>> Michael J Gruber writes:\n>>>\n>>>> - ce->ce_dev = st->st_dev;\n>>>> - ce->ce_ino = st->st_ino;\n>>>> - ce->ce_uid = st->st_uid;\n>>>> - ce->ce_gid = st->st_gid;\n>>>> - ce->ce_size = st->st_size;\n>>>> + ce->ce_dev = (unsigned int)st->st_dev;\n>>>> + ce->ce_ino = (unsigned int)st->st_ino;\n>>>> + ce->ce_uid = (unsigned int)st->st_uid;\n>>>> + ce->ce_gid = (unsigned int)st->st_gid;\n>>>> + ce->ce_size = (unsigned int)st->st_size;\n>>>\n>>> I haven't had my morning coffee yet, but wouldn't the conversion be\n>>> automatic by assignment anyway?\n>>\n>> Well, we do cast from dev_t etc. to unsigned int in all other places in\n>> read_cache.c.\n>>\n> \n> The only cases where we cast anything to unsigned in on assignment in\n> read-cache.h is where we assign the second-part of the ce_ctime and\n> ce_mtime members:\n> \tce->ce_ctime.sec = (unsigned int)st->st_ctime;\n> \tce->ce_mtime.sec = (unsigned int)st->st_mtime;\n> \n> These are indeed superfluous (as yours are) as far as I can tell.\n> There's also some casts on comparison, and I think they also are\n> superfluous. The current casts originate from commits fba2f38a,\n> 5bcf109c, e1afca4f and 7a51ed66.\n> \n> They don't do any harm though, so I don't think it makes much sense to\n> remove them.\n> \n\nOK, then we can leave them resp. leave them out until a compiler complains.\n\nI just stumbled upon this \"inconsistency\" when hunting down that fs\nboundary warning.\n\nMichael\n"}]}