[Freeciv-Dev] Re: (PR#14783) bug: worklist_length: Assertion `pwl->length >= 0 && pwl->length < 16' failed.

["Christian Prochaska" <cp.ml.freeciv.dev@xxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=14783 >

Jason Short wrote:

>Well, the assertion is wrong.  Here is the right assertion.
>
>However it still crashes inside the GTK client.  I haven't been able to
>track it down.  Can you test it in the SDL client?
>
>-jason
>  
>
The SDL client didn't crash, but only because it handles the current
production separately from the worklist and therefore doesn't allow the
worklist to be filled up with more than MAX_LEN_WORKLIST - 1 items. But
while testing I found another little bug in the SDL client's worklist
code which the attached patch fixes.



diff -urN -X devel/diff_ignore devel_distclean/client/gui-sdl/wldlg.c 
devel/client/gui-sdl/wldlg.c
--- devel_distclean/client/gui-sdl/wldlg.c      2005-12-07 03:54:52.000000000 
+0100
+++ devel/client/gui-sdl/wldlg.c        2005-12-07 21:36:09.000000000 +0100
@@ -475,19 +475,19 @@
     /* remove widget from widget list */
     del_widget_from_vertical_scroll_widget_list(pEditor->pWork, pItem);
   } else {
-        /* change productions to first worklist element */
-        struct GUI *pBuf = pItem->prev;
-      change_production(pEditor->pCopy_WorkList->entries[0]);
-        worklist_advance(pEditor->pCopy_WorkList);
-        del_widget_from_vertical_scroll_widget_list(pEditor->pWork, pItem);
-        FREE(pBuf->data.ptr);
-        if(pBuf != pEditor->pWork->pBeginActiveWidgetList) {
-          do{
-           pBuf = pBuf->prev;
-           *((int *)pBuf->data.ptr) = *((int *)pBuf->data.ptr) - 1;
-          } while(pBuf != pEditor->pWork->pBeginActiveWidgetList);
-        }
-      }
+    /* change productions to first worklist element */
+    struct GUI *pBuf = pItem->prev;
+    change_production(pEditor->pCopy_WorkList->entries[0]);
+    worklist_advance(pEditor->pCopy_WorkList);
+    del_widget_from_vertical_scroll_widget_list(pEditor->pWork, pItem);
+    FREE(pBuf->data.ptr);
+    if(pBuf != pEditor->pWork->pBeginActiveWidgetList) {
+      do{
+        pBuf = pBuf->prev;
+        *((int *)pBuf->data.ptr) = *((int *)pBuf->data.ptr) - 1;
+      } while(pBuf != pEditor->pWork->pBeginActiveWidgetList);
+    }
+  }
 
 /* FIXME: fix scrollbar code */
 #if 0    
@@ -540,6 +540,7 @@
   pText = pItem->string16->text;
   ID = pItem->ID;
   
+  /* second item or higher was clicked */
   if(pItem->data.ptr) {
     /* worklist operations -> swap down */
     int row = *((int *)pItem->data.ptr);
@@ -550,7 +551,7 @@
       
     changed = TRUE;  
   } else {
-    /* change production ... */
+    /* first item was clicked -> change production */
     change_production(pEditor->pCopy_WorkList->entries[0]);
     pEditor->pCopy_WorkList->entries[0] = cid_decode(MAX_ID - ID);
       changed = TRUE;
@@ -588,12 +589,14 @@
   Uint16 ID = pItem->ID;
   bool changed = FALSE;
   struct city_production tmp;  
-  
+
+  /* first item was clicked -> remove */
   if(pItem == pEditor->pWork->pEndActiveWidgetList) {
     remove_item_from_worklist(pItem);
     return;
   }
-    
+
+  /* third item or higher was clicked */  
   if(pItem->data.ptr && *((int *)pItem->data.ptr) > 0) {
     /* worklist operations -> swap up*/
     int row = *((int *)pItem->data.ptr);
@@ -604,13 +607,14 @@
       
     changed = TRUE;    
   } else {
-      /* change production ... */
+    /* second item was clicked -> change production ... */
+    tmp = pEditor->currently_building;
     change_production(pEditor->pCopy_WorkList->entries[0]);
-    pEditor->pCopy_WorkList->entries[0] = pEditor->currently_building;
+    pEditor->pCopy_WorkList->entries[0] = tmp;    
       
-      changed = FALSE;
-    }
-  
+    changed = TRUE;
+  }
+
   if(changed) {
     pItem->string16->text = pItem->next->string16->text;
     pItem->ID = pItem->next->ID;