Thread (1 message) 1 message, 1 author, 2020-11-10

Re: [PATCH v3] diff: make diff_free_filespec_data accept NULL

From: Junio C Hamano <hidden>
Date: 2020-11-10 17:02:22

Johannes Schindelin [off-list ref] writes:
quoted
 void diff_free_filespec_data(struct diff_filespec *s)
 {
+	if (!s)
+		return;
+
 	diff_free_filespec_blob(s);
 	FREE_AND_NULL(s->cnt_data);
We can write this in a more canonical way without wasting all that many
lines:

	if (s) {
		diff_free_filespec_blob(s);
		FREE_AND_NULL(s->cnt_data);
	}
On only this part.  Please do not use this one, as what was posted
is better.

By making it clear that we do not do anything when given NULL
upfront, it lets readers concentrate on the main part of the
function---"now we are done with NULL, what happens on a real case?"
And when readers are doing so, one extra indentation level is just
an extra noise that does not help.

In this particular case, it is important more from coding discipline
than anything else---for a small enough function like this one whose
main part fits on less than fourth of a screen, extra indentation
does not matter, but all code tends to grow and it starts to matter
if/when we need to clean more things in the function.

Everything else said in the review was excellent and very helpful.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help