From 1c1909d5fea376ed40d4bbd1cbfd72177212ce1f Mon Sep 17 00:00:00 2001 From: Christophe CURIS Date: Fri, 8 May 2015 13:19:04 +0200 Subject: [PATCH] wmaker: took as much assignation as possible outside 'if' statements It is generally considered bad practice to place an assignation inside the expression for an "if" statement because it is often a source of bug, because of possible typos and because it makes reviewing code more complicated. This patch fixes as much cases as possible to make the code easier to read. Signed-off-by: Christophe CURIS --- src/appmenu.c | 3 ++- src/dialog.c | 9 ++++++--- src/dock.c | 30 +++++++++++++++++++++--------- src/event.c | 6 ++++-- src/monitor.c | 3 ++- src/osdep_bsd.c | 9 ++++++--- src/osdep_linux.c | 6 ++++-- src/screen.c | 21 +++++++++++---------- src/session.c | 26 ++++++++++++++++++-------- src/wdefaults.c | 6 ++++-- src/winspector.c | 6 ++++-- src/workspace.c | 10 ++++++---- 12 files changed, 88 insertions(+), 47 deletions(-) diff --git a/src/appmenu.c b/src/appmenu.c index 3a08e62d..bc708964 100644 --- a/src/appmenu.c +++ b/src/appmenu.c @@ -139,7 +139,8 @@ static WMenu *parseMenuCommand(WScreen * scr, Window win, char **slist, int coun } wstrlcpy(title, &slist[*index][pos], sizeof(title)); } - if (!(data = malloc(sizeof(WAppMenuData)))) { + data = malloc(sizeof(WAppMenuData)); + if (data == NULL) { wwarning(_("appmenu: out of memory creating menu for window %lx"), win); wMenuDestroy(menu, True); return NULL; diff --git a/src/dialog.c b/src/dialog.c index 56cd3dba..cb444eeb 100644 --- a/src/dialog.c +++ b/src/dialog.c @@ -262,7 +262,8 @@ ScanFiles(const char *dir, const char *prefix, unsigned acceptmask, unsigned dec char *fullfilename, *suffix; prefixlen = strlen(prefix); - if ((d = opendir(dir)) != NULL) { + d = opendir(dir); + if (d != NULL) { while ((de = readdir(d)) != NULL) { if (strlen(de->d_name) > prefixlen && !strncmp(prefix, de->d_name, prefixlen) && @@ -298,12 +299,14 @@ static WMArray *GenerateVariants(const char *complete) while (*complete == ' ') ++complete; - if ((pos = strrchr(complete, ' ')) != NULL) { + pos = strrchr(complete, ' '); + if (pos != NULL) { complete = pos + 1; firstWord = False; } - if ((pos = strrchr(complete, '/')) != NULL) { + pos = strrchr(complete, '/'); + if (pos != NULL) { tmp = wstrndup((char *)complete, pos - complete + 1); if (*tmp == '~' && *(tmp + 1) == '/' && getenv("HOME")) { dir = wstrdup(getenv("HOME")); diff --git a/src/dock.c b/src/dock.c index d4aca092..e9851525 100644 --- a/src/dock.c +++ b/src/dock.c @@ -1530,7 +1530,8 @@ static WMPropList *dockSaveState(WDock *dock) if (!btn || btn->attracted) continue; - if ((icon_info = make_icon_state(dock->icon_array[i]))) { + icon_info = make_icon_state(dock->icon_array[i]); + if (icon_info != NULL) { WMAddToPLArray(list, icon_info); WMReleasePropList(icon_info); } @@ -2042,7 +2043,9 @@ static WDock *findDock(WScreen *scr, XEvent *event, int *icon_pos) int i; *icon_pos = -1; - if ((dock = scr->dock) != NULL) { + + dock = scr->dock; + if (dock != NULL) { for (i = 0; i < dock->max_icons; i++) { if (dock->icon_array[i] && dock->icon_array[i]->icon->core->window == event->xclient.window) { @@ -2669,7 +2672,8 @@ Bool wDockSnapIcon(WDock *dock, WAppIcon *icon, int req_x, int req_y, int *ret_x * it wants to be (ex_x) and slide them. */ j = 0; for (i = 1; i < dock->max_icons; i++) { - if ((aicon = dock->icon_array[ i ]) && aicon != icon && + aicon = dock->icon_array[i]; + if ((aicon != NULL) && (aicon != icon) && ((ex_x <= aicon->xindex && aicon->xindex < index_of_hole) || (index_of_hole < aicon->xindex && aicon->xindex <= ex_x))) aicons_to_shift[ j++ ] = aicon; @@ -3048,7 +3052,8 @@ static pid_t execCommand(WAppIcon *btn, const char *command, WSavedState *state) return 0; } - if ((pid = fork()) == 0) { + pid = fork(); + if (pid == 0) { char **args; int i; @@ -3752,7 +3757,9 @@ static void handleDockMove(WDock *dock, WAppIcon *aicon, XEvent *event) { for (i = 0; i < dock->max_icons; i++) { int new_y, new_index, j, ok; - if ((tmpaicon = dock->icon_array[i]) == NULL) + + tmpaicon = dock->icon_array[i]; + if (tmpaicon == NULL) continue; if (onScreen(scr, tmpaicon->x_pos, tmpaicon->y_pos)) continue; @@ -3830,7 +3837,8 @@ static void handleDockMove(WDock *dock, WAppIcon *aicon, XEvent *event) Window *wins[dock->icon_count]; for (i = 0; i < dock->max_icons; i++) { - if ((tmpaicon = dock->icon_array[i]) == NULL) + tmpaicon = dock->icon_array[i]; + if (tmpaicon == NULL) continue; wins[ tmpaicon->xindex + (dock->on_right_side ? dock->icon_count - 1 : 0) ] = &tmpaicon->icon->core->window; } @@ -4420,7 +4428,8 @@ static void drawerDestroy(WDock *drawer) if (drawer->icon_count == 2) { /* Drawer contains a single appicon: dock it where the drawer was */ for (i = 1; i < drawer->max_icons; i++) { - if ((aicon = drawer->icon_array[i])) + aicon = drawer->icon_array[i]; + if (aicon != NULL) break; } @@ -4431,7 +4440,8 @@ static void drawerDestroy(WDock *drawer) } else if (drawer->icon_count > 2) { icons = WMCreateArray(drawer->icon_count - 1); for (i = 1; i < drawer->max_icons; i++) { - if (!(aicon = drawer->icon_array[i])) + aicon = drawer->icon_array[i]; + if (aicon == NULL) continue; WMAddToArray(icons, aicon); } @@ -4559,7 +4569,9 @@ static void swapDrawer(WDock *drawer, int new_x) for (i = 0; i < drawer->max_icons; i++) { WAppIcon *ai; - if ((ai = drawer->icon_array[i]) == NULL) + + ai = drawer->icon_array[i]; + if (ai == NULL) continue; ai->xindex *= -1; /* so A B C becomes C B A */ ai->x_pos = new_x + ai->xindex * ICON_SIZE; diff --git a/src/event.c b/src/event.c index ce316eee..dbb927bd 100644 --- a/src/event.c +++ b/src/event.c @@ -582,7 +582,8 @@ static void handleMapRequest(XEvent * ev) WScreen *scr = NULL; Window window = ev->xmaprequest.window; - if ((wwin = wWindowFor(window))) { + wwin = wWindowFor(window); + if (wwin != NULL) { if (wwin->flags.shaded) { wUnshadeWindow(wwin); } @@ -931,7 +932,8 @@ static void handleConfigureRequest(XEvent * event) { WWindow *wwin; - if (!(wwin = wWindowFor(event->xconfigurerequest.window))) { + wwin = wWindowFor(event->xconfigurerequest.window); + if (wwin == NULL) { /* * Configure request for unmapped window */ diff --git a/src/monitor.c b/src/monitor.c index f06ff517..c784dd3f 100644 --- a/src/monitor.c +++ b/src/monitor.c @@ -103,7 +103,8 @@ int MonitorLoop(int argc, char **argv) } do { - if ((exited = waitpid(-1, &status, 0)) < 0) { + exited = waitpid(-1, &status, 0); + if (exited < 0) { werror(_("Error during monitoring of Window Maker process.")); error = True; break; diff --git a/src/osdep_bsd.c b/src/osdep_bsd.c index ad504adb..992f0617 100644 --- a/src/osdep_bsd.c +++ b/src/osdep_bsd.c @@ -102,17 +102,20 @@ Bool GetCommandForPid(int pid, char ***argv, int *argc) #if defined( OPENBSD ) /* kvm descriptor */ - if ((kd = kvm_openfiles(NULL, NULL, NULL, KVM_NO_FILES, kvmerr)) == NULL) + kd = kvm_openfiles(NULL, NULL, NULL, KVM_NO_FILES, kvmerr); + if (kd == NULL) return False; procs = 0; /* the process we are interested in */ - if ((kp = kvm_getprocs(kd, KERN_PROC_PID, pid, sizeof(*kp), &procs)) == NULL || procs == 0) + kp = kvm_getprocs(kd, KERN_PROC_PID, pid, sizeof(*kp), &procs); + if (kp == NULL || procs == 0) /* if kvm_getprocs() bombs out or does not find the process */ return False; /* get its argv */ - if ((nargv = kvm_getargv(kd, kp, 0)) == NULL) + nargv = kvm_getargv(kd, kp, 0); + if (nargv == NULL) return False; /* flatten nargv into args */ diff --git a/src/osdep_linux.c b/src/osdep_linux.c index bb1ef2e6..890ba6a2 100644 --- a/src/osdep_linux.c +++ b/src/osdep_linux.c @@ -38,7 +38,8 @@ Bool GetCommandForPid(int pid, char ***argv, int *argc) while (1) { /* not switching this to stdio yet, as this does not need * to be portable, and i'm lazy */ - if ((fd = open(buf, O_RDONLY)) != -1) + fd = open(buf, O_RDONLY); + if (fd != -1) break; if (errno == EINTR) continue; @@ -46,7 +47,8 @@ Bool GetCommandForPid(int pid, char ***argv, int *argc) } while (1) { - if ((count = read(fd, buf, sizeof(buf))) != -1) + count = read(fd, buf, sizeof(buf)); + if (count != -1) break; if (errno == EINTR) continue; diff --git a/src/screen.c b/src/screen.c index 51e23178..10a1fc95 100644 --- a/src/screen.c +++ b/src/screen.c @@ -841,16 +841,16 @@ void wScreenSaveState(WScreen * scr) if (!wPreferences.flags.nodock) { wDockSaveState(scr, old_state); } else { - if ((foo = WMGetFromPLDictionary(old_state, dDock)) != NULL) { + foo = WMGetFromPLDictionary(old_state, dDock); + if (foo != NULL) WMPutInPLDictionary(scr->session_state, dDock, foo); - } } if (!wPreferences.flags.noclip) { wClipSaveState(scr); } else { - if ((foo = WMGetFromPLDictionary(old_state, dClip)) != NULL) { + foo = WMGetFromPLDictionary(old_state, dClip); + if (foo != NULL) WMPutInPLDictionary(scr->session_state, dClip, foo); - } } wWorkspaceSaveState(scr, old_state); @@ -858,21 +858,22 @@ void wScreenSaveState(WScreen * scr) if (!wPreferences.flags.nodrawer) { wDrawersSaveState(scr); } else { - if ((foo = WMGetFromPLDictionary(old_state, dDrawers)) != NULL) { + foo = WMGetFromPLDictionary(old_state, dDrawers); + if (foo != NULL) WMPutInPLDictionary(scr->session_state, dDrawers, foo); - } } if (wPreferences.save_session_on_exit) { wSessionSaveState(scr); } else { - if ((foo = WMGetFromPLDictionary(old_state, dApplications)) != NULL) { + foo = WMGetFromPLDictionary(old_state, dApplications); + if (foo != NULL) WMPutInPLDictionary(scr->session_state, dApplications, foo); - } - if ((foo = WMGetFromPLDictionary(old_state, dWorkspace)) != NULL) { + + foo = WMGetFromPLDictionary(old_state, dWorkspace); + if (foo != NULL) WMPutInPLDictionary(scr->session_state, dWorkspace, foo); - } } /* clean up */ diff --git a/src/session.c b/src/session.c index 2af8fd09..90109a3f 100644 --- a/src/session.c +++ b/src/session.c @@ -128,9 +128,9 @@ static int getBool(WMPropList * value) if (!WMIsPLString(value)) { return 0; } - if (!(val = WMGetFromPLString(value))) { + val = WMGetFromPLString(value); + if (val == NULL) return 0; - } if ((val[1] == '\0' && (val[0] == 'y' || val[0] == 'Y')) || strcasecmp(val, "YES") == 0) { @@ -299,7 +299,8 @@ void wSessionSaveState(WScreen * scr) || WFLAGP(wwin, shared_appicon)) && !WFLAGP(wwin, dont_save_session)) { /* A entry for this application was not yet saved. Save one. */ - if ((win_info = makeWindowState(wwin, wapp)) != NULL) { + win_info = makeWindowState(wwin, wapp); + if (win_info != NULL) { WMAddToPLArray(list, win_info); WMReleasePropList(win_info); /* If we were succesful in saving the info for this window @@ -346,7 +347,8 @@ static pid_t execCommand(WScreen *scr, char *command) return 0; } - if ((pid = fork()) == 0) { + pid = fork(); + if (pid == 0) { char **args; int i; @@ -392,13 +394,21 @@ static WSavedState *getWindowState(WScreen * scr, WMPropList * win_state) state->workspace--; } } - if ((value = WMGetFromPLDictionary(win_state, sShaded)) != NULL) + + value = WMGetFromPLDictionary(win_state, sShaded); + if (value != NULL) state->shaded = getBool(value); - if ((value = WMGetFromPLDictionary(win_state, sMiniaturized)) != NULL) + + value = WMGetFromPLDictionary(win_state, sMiniaturized); + if (value != NULL) state->miniaturized = getBool(value); - if ((value = WMGetFromPLDictionary(win_state, sHidden)) != NULL) + + value = WMGetFromPLDictionary(win_state, sHidden); + if (value != NULL) state->hidden = getBool(value); - if ((value = WMGetFromPLDictionary(win_state, sShortcutMask)) != NULL) { + + value = WMGetFromPLDictionary(win_state, sShortcutMask); + if (value != NULL) { mask = getInt(value); state->window_shortcuts = mask; } diff --git a/src/wdefaults.c b/src/wdefaults.c index c2d9eb80..b57b0167 100644 --- a/src/wdefaults.c +++ b/src/wdefaults.c @@ -568,14 +568,16 @@ void wDefaultChangeIcon(const char *instance, const char *class, const char *fil icon_value = WMCreatePLDictionary(AIcon, value, NULL); WMReleasePropList(value); - if ((def_win = WMGetFromPLDictionary(dict, AnyWindow)) != NULL) + def_win = WMGetFromPLDictionary(dict, AnyWindow); + if (def_win != NULL) def_icon = WMGetFromPLDictionary(def_win, AIcon); if (def_icon && !strcmp(WMGetFromPLString(def_icon), file)) same = 1; } - if ((attr = WMGetFromPLDictionary(dict, key)) != NULL) { + attr = WMGetFromPLDictionary(dict, key); + if (attr != NULL) { if (WMIsPLDictionary(attr)) { if (icon_value != NULL && !same) WMMergePLDictionaries(attr, icon_value, False); diff --git a/src/winspector.c b/src/winspector.c index a3c384fa..78b5cd0f 100644 --- a/src/winspector.c +++ b/src/winspector.c @@ -376,7 +376,8 @@ static int getBool(WMPropList *value) if (!WMIsPLString(value)) return 0; - if (!(val = WMGetFromPLString(value))) + val = WMGetFromPLString(value); + if (val == NULL) return 0; if ((val[1] == '\0' && @@ -407,7 +408,8 @@ insertAttribute(WMPropList *dict, WMPropList *window, WMPropList *attr, WMPropLi int update = 0, modified = 0; if (!(flags & UPDATE_DEFAULTS) && dict) { - if ((def_win = WMGetFromPLDictionary(dict, AnyWindow)) != NULL) + def_win = WMGetFromPLDictionary(dict, AnyWindow); + if (def_win != NULL) def_value = WMGetFromPLDictionary(def_win, attr); } diff --git a/src/workspace.c b/src/workspace.c index 69db16cf..c6052b3f 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -492,7 +492,8 @@ void wWorkspaceForceChange(WScreen * scr, int workspace) wWorkspaceMenuUpdate(scr, scr->clip_ws_menu); - if ((tmp = scr->focused_window) != NULL) { + tmp = scr->focused_window; + if (tmp != NULL) { WWindow **toUnmap; int toUnmapSize, toUnmapCount; @@ -856,10 +857,11 @@ void wWorkspaceSaveState(WScreen * scr, WMPropList * old_state) WMPutInPLDictionary(wks_state, dClip, pstr); WMReleasePropList(pstr); } else if (old_wks_state != NULL) { - if ((foo = WMGetFromPLArray(old_wks_state, i)) != NULL) { - if ((bar = WMGetFromPLDictionary(foo, dClip)) != NULL) { + foo = WMGetFromPLArray(old_wks_state, i); + if (foo != NULL) { + bar = WMGetFromPLDictionary(foo, dClip); + if (bar != NULL) WMPutInPLDictionary(wks_state, dClip, bar); - } } } WMAddToPLArray(parr, wks_state);