cleanup and code deduplication

* ClientPattern.cc: make compiler happy (complaints about XPROP not handled in switch)
* FbTk/FbPixmap.cc FbTk/StringUtil.cc: signed vs unsigned
* FbTk/FbString.cc: missing return
* WinClient.cc: create atoms only once; use helper function
* Window.cc: use a helper function

src/ClientPattern.cc

@@ -373,7 +373,7 @@ bool ClientPattern::addTerm(const FbTk::FbString &str, WinProperty prop, bool ne
     if (!term)
         return rc;
 
-    if (rc = !term->regexp.error()) {
+    if ((rc = !term->regexp.error())) {
         m_terms.push_back(term);
     } else {
         delete term;
@@ -383,76 +383,84 @@ bool ClientPattern::addTerm(const FbTk::FbString &str, WinProperty prop, bool ne
 }
 
 FbTk::FbString ClientPattern::getProperty(WinProperty prop, const Focusable &client) {
+
+    FbTk::FbString result;
+
     // we need this for some of the window properties
     const FluxboxWindow *fbwin = client.fbwindow();
 
     switch (prop) {
     case TITLE:
-        return client.title().logical();
+        result = client.title().logical();
         break;
     case CLASS:
-        return client.getWMClassClass();
+        result = client.getWMClassClass();
-        break;
-    case NAME:
-        return client.getWMClassName();
         break;
     case ROLE:
-        return client.getWMRole();
+        result = client.getWMRole();
         break;
     case TRANSIENT:
-        return client.isTransient() ? "yes" : "no";
+        result = client.isTransient() ? "yes" : "no";
         break;
     case MAXIMIZED:
-        return (fbwin && fbwin->isMaximized()) ? "yes" : "no";
+        result = (fbwin && fbwin->isMaximized()) ? "yes" : "no";
         break;
     case MINIMIZED:
-        return (fbwin && fbwin->isIconic()) ? "yes" : "no";
+        result = (fbwin && fbwin->isIconic()) ? "yes" : "no";
         break;
     case SHADED:
-        return (fbwin && fbwin->isShaded()) ? "yes" : "no";
+        result = (fbwin && fbwin->isShaded()) ? "yes" : "no";
         break;
     case STUCK:
-        return (fbwin && fbwin->isStuck()) ? "yes" : "no";
+        result = (fbwin && fbwin->isStuck()) ? "yes" : "no";
         break;
     case FOCUSHIDDEN:
-        return (fbwin && fbwin->isFocusHidden()) ? "yes" : "no";
+        result = (fbwin && fbwin->isFocusHidden()) ? "yes" : "no";
         break;
     case ICONHIDDEN:
-        return (fbwin && fbwin->isIconHidden()) ? "yes" : "no";
+        result = (fbwin && fbwin->isIconHidden()) ? "yes" : "no";
         break;
     case WORKSPACE: {
         unsigned int wsnum = (fbwin ? fbwin->workspaceNumber() : client.screen().currentWorkspaceID());
-        return FbTk::StringUtil::number2String(wsnum);
+        result = FbTk::StringUtil::number2String(wsnum);
         break;
     }
     case WORKSPACENAME: {
         const Workspace *w = (fbwin ?
                 client.screen().getWorkspace(fbwin->workspaceNumber()) :
                 client.screen().currentWorkspace());
-        return w ? w->name() : "";
+        if (w) {
+            result = w->name();
+        }
         break;
     }
-    case HEAD: {
+    case HEAD:
-        if (!fbwin)
+        if (fbwin) {
-            return "";
+            result = FbTk::StringUtil::number2String(client.screen().getHead(fbwin->fbWindow()));
-        int head = client.screen().getHead(fbwin->fbWindow());
+        }
-        return FbTk::StringUtil::number2String(head);
         break;
-    }
     case LAYER:
-        return fbwin ? ::Layer::getString(fbwin->layerNum()) : "";
+        if (fbwin) {
+            result = ::Layer::getString(fbwin->layerNum());
+        }
         break;
     case URGENT:
-        return Fluxbox::instance()->attentionHandler()
+        result = Fluxbox::instance()->attentionHandler()
                 .isDemandingAttention(client) ? "yes" : "no";
         break;
-    case SCREEN: {
+    case SCREEN:
-        int screenId = client.screen().screenNumber();
+        result = FbTk::StringUtil::number2String(client.screen().screenNumber());
-        return FbTk::StringUtil::number2String(screenId);
+        break;
+
+    case XPROP:
+        break;
+
+    case NAME:
+    default:
+        result = client.getWMClassName();
         break;
     }
-    }
+    return result;
-    return client.getWMClassName();
 }
 
 bool ClientPattern::operator ==(const ClientPattern &pat) const {

src/FbTk/FbPixmap.cc

@@ -369,7 +369,7 @@ bool FbPixmap::rootwinPropertyNotify(int screen_num, Atom atom) {
         return false;
 
     checkAtoms();
-    for (int i=0; i < sizeof(root_props)/sizeof(RootProps); ++i) {
+    for (size_t i = 0; i < sizeof(root_props)/sizeof(RootProps); ++i) {
         if (root_props[i].atom == atom) {
             Pixmap root_pm = None;
             Atom real_type;

src/FbTk/FbString.cc

@@ -126,6 +126,7 @@ const FbString& BiDiString::setLogical(const FbString& logical) {
         m_visual_dirty = true;
     }
 #endif
+    return m_logical;
 }
 
 const FbString& BiDiString::visual() const {

src/FbTk/StringUtil.cc

@@ -183,7 +183,7 @@ string findExtension(const string &filename) {
 string::size_type findCharFromAlphabetAfterTrigger(const std::string& in, char trigger, const char alphabet[], size_t len_alphabet, size_t* found) {
     for (const char* s = in.c_str(); *s != '\0'; ) {
         if (*s++ == trigger && *s != '\0') {
-            for (const char* a = alphabet; (a - alphabet) < len_alphabet; ++a) {
+            for (const char* a = alphabet; (a - alphabet) < static_cast<ssize_t>(len_alphabet); ++a) {
                 if (*s == *a) {
                     if (found) {
                         *found = a - alphabet;

src/Window.cc

@@ -133,6 +133,16 @@ WinClient *getRootTransientFor(WinClient *client) {
 }
 
 
+void callForAllTransient(FluxboxWindow& win, void (*callFunc)(FluxboxWindow&)) {
+    WinClient::TransientList::const_iterator it = win.winClient().transientList().begin();
+    WinClient::TransientList::const_iterator it_end = win.winClient().transientList().end();
+    for (; it != it_end; ++it) {
+        if ((*it)->fbwindow() && !(*it)->fbwindow()->isIconic())
+            // TODO: should we also check if it is the active client?
+            callFunc(*(*it)->fbwindow());
+    }
+}
+
 /// raise window and do the same for each transient of the current window
 void raiseFluxboxWindow(FluxboxWindow &win) {
     if (win.oplock)
@@ -150,19 +160,10 @@ void raiseFluxboxWindow(FluxboxWindow &win) {
 
     win.layerItem().raise();
 
-    // for each transient do raise
+    callForAllTransient(win, raiseFluxboxWindow);
-
-    WinClient::TransientList::const_iterator it = win.winClient().transientList().begin();
-    WinClient::TransientList::const_iterator it_end = win.winClient().transientList().end();
-    for (; it != it_end; ++it) {
-        if ((*it)->fbwindow() && !(*it)->fbwindow()->isIconic())
-            // TODO: should we also check if it is the active client?
-            raiseFluxboxWindow(*(*it)->fbwindow());
-    }
 
     win.oplock = false;
 
-
     if (!win.winClient().transientList().empty())
         win.screen().layerManager().unlock();
 
@@ -210,16 +211,9 @@ void tempRaiseFluxboxWindow(FluxboxWindow &win) {
         win.layerItem().tempRaise();
     }
 
-    // for each transient do raise
+    callForAllTransient(win, tempRaiseFluxboxWindow);
-    WinClient::TransientList::const_iterator it = win.winClient().transientList().begin();
-    WinClient::TransientList::const_iterator it_end = win.winClient().transientList().end();
-    for (; it != it_end; ++it) {
-        if ((*it)->fbwindow() && !(*it)->fbwindow()->isIconic())
-            // TODO: should we also check if it is the active client?
-            tempRaiseFluxboxWindow(*(*it)->fbwindow());
-    }
-    win.oplock = false;
 
+    win.oplock = false;
 }
 
 class SetClientCmd:public FbTk::Command<void> {
@@ -1063,8 +1057,8 @@ void FluxboxWindow::reconfigure() {
     m_timer.setTimeout(Fluxbox::instance()->getAutoRaiseDelay());
 
     updateButtons();
-    frame().reconfigure();
 
+    frame().reconfigure();
     menu().reconfigure();
 
     Client2ButtonMap::iterator it = m_labelbuttons.begin(),