@@ -23,10 +23,6 @@ Data Structures Functions ----------`init_hash`::-- Initialize the hash table.- `free_hash`:: Release memory associated with the hash table.
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..)
On Mon, Jul 26, 2010 at 10:36 PM, Stephen Boyd [off-list ref] wrote:
init_hash() is essentially a memset() so just use that.
I disagree.
Yes, technically right now the initialization just zeroes out all the
fields, and memset() does the same. But there is no advantage to using
memset(), since init_hash() will create the same or better code, and
using init_hash() is way more readable.
Also, it's not at all the case that init_hash() is always going to be
a memset(). Imagine a threaded hash-table with a lock associated with
it or something. Now, admittedly that's not necessarily something we'd
ever do in git, but I still think it's simply a good idea to have a
clear "initialize this" routine, rather than depending on the fact
that zeroing it out is sufficient.
Linus
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).
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.
-Peff
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 :)
From: Stephen Boyd <hidden> Date: 2016-06-15 22:49:12
On 07/27/2010 12:58 PM, Ævar Arnfjörð Bjarmason wrote:
quoted
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 :)
Ok, seems like nobody thinks this is a good idea so I'm fine with dropping it. The minimal savings (if at all) doesn't seem worth the code churn. One less patch in the queue ;-)