Re: [PATCH] hash: Remove useless init_hash()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 27, 2010 at 19:49, Jeff King <peff@xxxxxxxx> wrote:
> On Tue, Jul 27, 2010 at 10:36:09AM +0000, Ævar Arnfjörð Bjarmason wrote:
>
>> >> -static inline void init_hash(struct hash_table *table)
>> >> -{
>> >> -     table->size = 0;
>> >> -     table->nr = 0;
>> >> -     table->array = NULL;
>> >> -}
>> >
>> > *This* could be replaced by memset.
>>
>> No it couldn't? The second argument to memset is just an int, so
>> setting the memory area to 0 isn't portable to systems where the
>> representation of NULL isn't "0".
>>
>> (It's early so I may be misremembering my C..)
>
> You're remembering your C correctly. It isn't portable, but it is so
> unlikely on modern machines that we simply don't care (and you will see
> memsets zero-ing pointers like this all through the git code, so this is
> certainly not introducing anything new).

Thanks for the confirmation. I was aware that NULL != 0 only occured
on long-dead architechtures, but I suspected that some compiler out
there would whine if it could statically determine that you were
reading in a memzero'd area and using it as NULL. Evidently not, or at
least nobody's complained.

> That being said, I agree with the comments that removing init_hash
> actually makes the code _less_ readable. You could just replace these
> three lines with a memset, but why? It's just code churn.

Yeah, and for the record it also missed this part in hash.c:

    void free_hash(struct hash_table *table)
    {
    	free(table->array);
    	table->array = NULL;
    	table->size = 0;
    	table->nr = 0;
    }

Have fun everyone :)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]