Am 03.06.2013 00:38, schrieb Felipe Contreras:
On Sun, Jun 2, 2013 at 3:26 PM, René Scharfe
<rene.scharfe@xxxxxxxxxxxxxx> wrote:
Am 02.06.2013 19:59, schrieb Felipe Contreras:
On Sun, Jun 2, 2013 at 12:54 PM, René Scharfe
<rene.scharfe@xxxxxxxxxxxxxx> wrote:
Am 02.06.2013 19:25, schrieb Felipe Contreras:
On Sun, Jun 2, 2013 at 10:46 AM, René Scharfe
<rene.scharfe@xxxxxxxxxxxxxx> wrote:
+ for (i = 0; i < n; i++) {
+ struct cache_entry *ce = src[i + o->merge];
+ if (ce != o->df_conflict_entry)
It's possible that ce is NULL, but you didn't add that check because
free(NULL) still works? Or because ce cannot be NULL?
If it's the former, I think it's clearer if we check that ce is not
NULL either way.
It is NULL if one tree misses an entry (e.g. a new or removed file). free
handles NULL and we generally avoid duplicating its NULL-check.
Yeah, but I can see somebody adding code inside that 'if' clause to
print the cache entry, and see a crash only to wonder what's going on.
And to save what? 5 characters?
The person adding code that depends on ce not being NULL needs to add that
check as well. Let's not worry too much about future changes that may or
(more likely IMHO) may not be done. The test suite covers this case
multiple times, so such a mistake doesn't have a realistic chance to hit
master.
What do we gain by not doing this? 5 less characters?
By following the convention of not checking for NULL when freeing, we
reduce the size of the code slightly and have one less branch
instruction in the object code. That's not particularly exciting in a
single instance but makes a difference if followed throughout the code base.
What do we gain by adding a duplicate check? A few minutes of debugging
time by the person who will add some code and forget the NULL check?
And how likely is that to happen?
René
--
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