[PATCH v2 6/7] shell32/autocomplete: Use the optional IACList interface and IACList::Expand, if available

Huw Davies huw at codeweavers.com
Mon Oct 15 07:37:06 CDT 2018


On Mon, Oct 15, 2018 at 03:32:50PM +0300, Gabriel Ivăncescu wrote:
> On Mon, Oct 15, 2018 at 2:55 PM Huw Davies <huw at codeweavers.com> wrote:
> > On Mon, Oct 15, 2018 at 02:24:01PM +0300, Gabriel Ivăncescu wrote:
> > > On Mon, Oct 15, 2018 at 1:15 PM Huw Davies <huw at codeweavers.com> wrote:
> > > >
> > > > This is going to need some work; trying to follow this code is just too hard.
> > >
> > > I know I could comment it better, but I don't want to be too verbose
> > > so some advice would be appreciated.
> >
> > Well we've dicussed gotos already.  Admittedly this one isn't
> > inside a nested switch, but still.
> 
> But what's the problem with this goto? It's the same as failure gotos
> which are used a lot in Wine's codebase. I used it because to me it
> seems much cleaner of the purpose and the label's name (i.e. it
> clearly expands given the last_delim, and also jumps to the end of the
> function, no control flow issues).
> 
> The function itself is small and fits in one page so having an extra
> helper function for this seems quite a bit overkill and less elegant
> in encapsulation to me... (if C supported local/inner functions it
> wouldn't be less elegant but of course IMO)
> 
> I guess I could also duplicate the expand code but that's even worse
> for maintenance.

A helper function will be just fine.

Huw.



More information about the wine-devel mailing list