Thanks for the analysis Rodolfo, but I don't like your patch(es).
On Mon, 9 Sep 2013 at 1:12:50 +0200, Rodolfo García Peñas wrote:
> On Mon, 09 Sep 2013, Rodolfo kix Garcia escribió:
>
> > On Mon, 09 Sep 2013, Amadeusz Sławiński escribió:
> >
> > > Hello,
> > >
> > > I installed few dockapps and added them to dock, however when I edit
> > > their wmaker settings (even when I don't change anything) and click
> > > 'ok' it causes windowmaker to crash. For whatever reason it doesn't seem
> > > to happen with normal applications.
> > >
> > > I'm using freshly build wmaker from next branch. Only information
> > > appearing on console is following line:
> > > wmaker(MonitorLoop(monitor.c:132)): warning: Window Maker exited due to
> > > a crash (signal 11) and will be restarted.
> > > and then it restarts.
> > >
> > > Amadeusz
> >
> > Confirmed. I will check it now.
> >
> > kix
>
> Hi,
>
> my patch c21c396fd381abb6cf23f0ad070a08a2c08a95be (wIconChangeImageFile
> returns rigth values) is wrong.
What was the motivation for writing it in the first place? The commit
message doesn't tell us why you wanted to make the change.
> kix@osaka:~/src/wmaker/git/wmaker-crm/src$ grep wIconChangeImageFile *c |
> grep if
> appicon.c: if (wIconChangeImageFile(icon->icon, file) == False) {
> dockedapp.c: if (wIconChangeImageFile(panel->editedIcon->icon,
> text) == False) {
> kix@osaka:~/src/wmaker/git/wmaker-crm/src$
>
> The reason is because the function wIconChangeImageFile returns True when is
> OK,
> and False when it fails. When the filename field is empty, it returns
> False:
The traditional way is to return 0 on success and 1 otherwise, because
you can simply write
if (!wIconChangeImageFile(panel->editedIcon->icon, text))
to take action when the function "couldn't do" what is was supposed
to do. Your original patch changed it to
if (wIconChangeImageFile(panel->editedIcon->icon, text) == False)
while at the same swapping the return values of the function. That's
not good, as you pointed out yourself in this email.
BUT:
> @@ -541,7 +541,7 @@ static void setIconCallback(WMenu *menu, WMenuEntry
> *entry)
> wfree(file);
> file = NULL;
> }
> - if (wIconChangeImageFile(icon->icon, file) == False) {
> + if (wIconChangeImageFile(icon->icon, file)) {
> wMessageDialog(scr, _("Error"),
> _("Could not open specified icon file"),
> _("OK"), NULL, NULL);
> } else {
This is _not_ a good way to fix this. The original mistake should be
corrected and the traditional way to test failure should be restored.
What do you think about making wIconChangeImageFile() return 'int' and
get rid of the True/False and use 0 and 1?
Things like
if (something() == False)
are bad style compared to
if (!something())
--
To unsubscribe, send mail to [email protected].
