Re: Launch buttons in the options dialog now look for a locally installed app

In <[email protected]>
Tony Houghton <[email protected]> wrote:

> In <[email protected]>
> Stephen Watson <[email protected]> wrote:
> 
> > "Thomas Leonard" <[email protected]> wrote:
> > 
> > > Looking at commit 11ce57102f89a7dad707d1a2f050749c4507414f
> > > (kerofin):
> > > 
> > > +  for(i=0; search[i]; i++)
> > > +  {
> > > +	  app=g_strconcat(search[i], "/", appname, NULL);
> > > +	  if(access(app, X_OK)==0)
> > > +		  goto out;
>           g_free(app);
>           app = NULL;
> > > +  }
> > > 
> > > Missing a g_free?
> > 
> > I don't think so.  app is returned and search is freed before exit.
> 
> It needs to be freed each time the loop fails to find a match though,
> as I've inserted above. I'm not sure whether setting it to NULL is
> also necessary without seeing the rest of the code, but this is the
> safe option.

Actually where did the code come from? I can't find that commit in the
latest pull. I'd like to have a look at it and see if that goto is
really appropriate. goto does have its uses but they should be fairly
exceptional. It might be better for the code after the loop to be based
on:

if (app)
{
    ...
}
else
{
    ...
}

-- 
TH * http://www.realh.co.uk

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php