Re: [hackers] [st] [patch] use goto in xloadfonts

From: Roberto E. Vargas Caballero <k0ga_AT_shike2.com>
Date: Mon, 22 Jun 2015 09:26:17 +0200

Hi,

On Sat, Jun 20, 2015 at 01:32:45AM -0400, Michael Reed wrote:
> > You just made the programmflow harder to grasp and removed the possibility
> > to differentiate between the errors in the future. Also the patch adds 4
> > SLoC without achieving anything.
>
> I agree it's harder to grasp, but only slightly, and I think the decrease in
> redundancy is worth it (although that's evidently subjective).

I personally don't think is harder. I think is a common pattern in
C to have die calls in the end of the function and gotos to them.
The main problem that I can see here is the label, whose name is
not significative enough: err, which error?, and if we have to add
more errors in the future, do we have to change this label and all
the gotos to this label?. I think a label like 'cant_open_font' is
better. I usually like short names, but in the case of goto the
story is totally different, because you have a modification in the
control flow and it should be clear why it is done.

> I don't agree with the comment on error handling; it's not clear to me
> when/if this function will be refactored, but I don't know if gotos have
> complicated refactoring st in the past, so maybe you're right.
>

The problem here is different. St was broken in the past due to
style changes, and the history of the project is hard to read due
to this kind of patches, so several suckless developers agreed in
only accept patches which fix some error or add some feature. Style
changes will be accepted only if there is something else in the
patch. I like your patch (it makes st more similar to my style ;) ),
but due to this reason I will not apply it.


Regards,
Received on Mon Jun 22 2015 - 09:26:17 CEST