On Wed, 19 Jun 2013 at 15:34:02 +0300, Gabriel VLASIU wrote:
> 
> WindowMaker segfault when I use alt-tab and SwitchPanelImages is set to 
> None. That is because WMGetFromArray does not check if the array passed 
> as argument is NULL.
> Actually, none of the functions in array.c do that.
> 
> See attached patch.

Thanks for the patch.

I wonder why this bug has not been found before though, since it seems
old and fundamental. I assume that the option "None" worked at some
point in the past though, have you checked when the bug was introduced?

I'm very curious to understand why this was never detected before.


> 
> Backtrace:
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x00007fb0787db776 in WMGetFromArray (array=0x0, index=0) at array.c:220
> 220             return array->items[index];
> (gdb) bt
> #0  0x00007fb0787db776 in WMGetFromArray (array=0x0, index=0) at array.c:220
> #1  0x000000000045cef9 in changeImage (panel=0x105a870, idecks=0, selected=0, 
> dim=0, force=0) at switchpanel.c:115
> #2  0x000000000045eb40 in wSwitchPanelSelectNext (panel=0x105a870, back=0, 
> ignore_minimized=0, class_only=0)
>     at switchpanel.c:642
> #3  0x000000000041874e in StartWindozeCycle (wwin=0x105b130, 
> event=0x7fff31c8fd40, next=1, class_only=0)
>     at cycling.c:134
> #4  0x0000000000436293 in handleKeyPress (event=0x7fff31c8fd40) at 
> event.c:1575
> #5  0x000000000043282e in DispatchEvent (event=0x7fff31c8fd40) at event.c:238
> #6  0x00007fb078a250f4 in WMHandleEvent (event=0x7fff31c8fd40) at wevent.c:208
> #7  0x0000000000432b1d in EventLoop () at event.c:407
> #8  0x000000000043fc09 in real_main (argc=2, argv=0x7fff31c8ff88) at 
> main.c:842
> #9  0x000000000043f032 in main (argc=2, argv=0x7fff31c8ff88) at main.c:642
> 
> 
> Sincerely,
> Gabriel
> 
> - -- 
> 
> // Gabriel VLASIU
> //
> // OpenGPG-KeyID      : 44952F15
> // OpenGPG-Fingerprint: 4AC5 7C26 2FE9 02DA 4906  24B2 D32B 7ED7 4495 2F15
> // OpenGPG-URL        : http://www.vlasiu.net/public.key
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.13 (GNU/Linux)
> 
> iQIcBAEBCgAGBQJRwaU6AAoJENMrftdElS8VLwsP/3N2bCjtVUriTOENXhwrOnlw
> JeYzSBw9guvztlYzLIBNlhADyMKXYMGumJYIxD2kWmS7zjTBVqMrG710/B02SuoG
> hWxJ6osPhZYnMKSW13EvVXu3WhHByrquejkwqgVBhfyzRqOE4HJnJH/yYuI6pnwe
> WRtg9u4Bi62yx9kyFGNNRX0HFnhPmkNFsEf7Q3vkLeL372s97tLqN5zjnbkhhbiJ
> 2SvUDRchfpqRmCAnEUd0QdI2iZOS43G7xiTCXcWNDnz2+AnxpMkTX9X4DXxyEzPs
> XY1EarPXL5CvhGhMGAOEK5PROonrNF1GvaDymNzwByFrHQLA77VGz9VLD2/L7kk1
> gC7d/SNDXTuMg2he9zuj4YAZs5QneG7G8RbqjUANeJGgn+Fc9/mQpmHLSibPBYYZ
> N2t+jDz/e10UcGpRg4h1AeLMsa2L50eO/sboIHaJ9h5e2fn5i5wYFi8Ty3nZ8KaK
> hg2Ju7UENnqUVSv61wS3I9hxJdVb0CWAV89xmZn/A+/wu+I2Mq7Gw213Ig2XEWSo
> HeM2f5fAS2f6C0dsWI1kvhHt9lDH1HKFdawwoOZ+uKtMV685SdI06daCWhAEpgAy
> zAkFD2VJCuoXtVgw0gFAnuaBuOCNBA30i7KnFMmkMJ2VsYdsCFS8o/IINehQ4u/y
> 7cB9hfIOG2HNQhj38eaN
> =BdLf
> -----END PGP SIGNATURE-----

> From 7fa3546ced9230fe1edc2a35144f2bca4f3a4e46 Mon Sep 17 00:00:00 2001
> From: Gabriel VLASIU <[email protected]>
> Date: Wed, 19 Jun 2013 15:28:04 +0300
> Subject: [PATCH 1/1] Fix segfault when SwitchPanelImages = None and user press
>  Alt+tab.
> 
> ---
>  WINGs/array.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/WINGs/array.c b/WINGs/array.c
> index ae1e17c..390df02 100644
> --- a/WINGs/array.c
> +++ b/WINGs/array.c
> @@ -77,6 +77,9 @@ void WMEmptyArray(WMArray * array)
>  
>  void WMFreeArray(WMArray * array)
>  {
> +     if (array == NULL)
> +             return;
> +
>       WMEmptyArray(array);
>       wfree(array->items);
>       wfree(array);
> @@ -84,11 +87,17 @@ void WMFreeArray(WMArray * array)
>  
>  int WMGetArrayItemCount(WMArray * array)
>  {
> +     if (array == NULL)
> +             return 0;
> +
>       return array->itemCount;
>  }
>  
>  void WMAppendArray(WMArray * array, WMArray * other)
>  {
> +     if (array == NULL || other == NULL)
> +             return;
> +
>       if (other->itemCount == 0)
>               return;
>  
> @@ -103,6 +112,9 @@ void WMAppendArray(WMArray * array, WMArray * other)
>  
>  void WMAddToArray(WMArray * array, void *item)
>  {
> +     if (array == NULL)
> +             return;
> +
>       if (array->itemCount >= array->allocSize) {
>               array->allocSize += RESIZE_INCREMENT;
>               array->items = wrealloc(array->items, sizeof(void *) * 
> array->allocSize);
> @@ -116,6 +128,9 @@ void WMInsertInArray(WMArray * array, int index, void 
> *item)
>  {
>       wassertr(index >= 0 && index <= array->itemCount);
>  
> +     if (array == NULL)
> +             return;
> +
>       if (array->itemCount >= array->allocSize) {
>               array->allocSize += RESIZE_INCREMENT;
>               array->items = wrealloc(array->items, sizeof(void *) * 
> array->allocSize);
> @@ -135,6 +150,9 @@ void *WMReplaceInArray(WMArray * array, int index, void 
> *item)
>  
>       wassertrv(index >= 0 && index <= array->itemCount, NULL);
>  
> +     if (array == NULL)
> +             return NULL;
> +
>       /* is it really useful to perform append if index == array->itemCount ? 
> -Dan */
>       if (index == array->itemCount) {
>               WMAddToArray(array, item);
> @@ -151,6 +169,9 @@ int WMDeleteFromArray(WMArray * array, int index)
>  {
>       wassertrv(index >= 0 && index < array->itemCount, 0);
>  
> +     if (array == NULL)
> +             return 0;
> +
>       if (array->destructor) {
>               array->destructor(array->items[index]);
>       }
> @@ -169,6 +190,9 @@ int WMRemoveFromArrayMatching(WMArray * array, 
> WMMatchDataProc * match, void *cd
>  {
>       int i;
>  
> +     if (array == NULL)
> +             return 1;
> +
>       if (match != NULL) {
>               for (i = 0; i < array->itemCount; i++) {
>                       if ((*match) (array->items[i], cdata)) {
> @@ -190,7 +214,7 @@ int WMRemoveFromArrayMatching(WMArray * array, 
> WMMatchDataProc * match, void *cd
>  
>  void *WMGetFromArray(WMArray * array, int index)
>  {
> -     if (index < 0 || index >= array->itemCount)
> +     if (index < 0 || array == NULL || index >= array->itemCount)
>               return NULL;
>  
>       return array->items[index];
> @@ -198,7 +222,7 @@ void *WMGetFromArray(WMArray * array, int index)
>  
>  void *WMPopFromArray(WMArray * array)
>  {
> -     if (array->itemCount <= 0)
> +     if (array->itemCount <= 0 || array == NULL)
>               return NULL;
>  
>       array->itemCount--;
> @@ -210,6 +234,9 @@ int WMFindInArray(WMArray * array, WMMatchDataProc * 
> match, void *cdata)
>  {
>       int i;
>  
> +     if (array == NULL)
> +             return WANotFound;
> +
>       if (match != NULL) {
>               for (i = 0; i < array->itemCount; i++) {
>                       if ((*match) (array->items[i], cdata))
> @@ -229,6 +256,9 @@ int WMCountInArray(WMArray * array, void *item)
>  {
>       int i, count;
>  
> +     if (array == NULL)
> +             return 0;
> +
>       for (i = 0, count = 0; i < array->itemCount; i++) {
>               if (array->items[i] == item)
>                       count++;
> @@ -239,6 +269,9 @@ int WMCountInArray(WMArray * array, void *item)
>  
>  void WMSortArray(WMArray * array, WMCompareDataProc * comparer)
>  {
> +     if (array == NULL)
> +             return;
> +
>       if (array->itemCount > 1) {     /* Don't sort empty or single element 
> arrays */
>               qsort(array->items, array->itemCount, sizeof(void *), comparer);
>       }
> @@ -248,6 +281,9 @@ void WMMapArray(WMArray * array, void (*function) (void 
> *, void *), void *data)
>  {
>       int i;
>  
> +     if (array == NULL)
> +             return;
> +
>       for (i = 0; i < array->itemCount; i++) {
>               (*function) (array->items[i], data);
>       }
> @@ -257,7 +293,7 @@ WMArray *WMGetSubarrayWithRange(WMArray * array, WMRange 
> aRange)
>  {
>       WMArray *newArray;
>  
> -     if (aRange.count <= 0)
> +     if (aRange.count <= 0 || array == NULL)
>               return WMCreateArray(0);
>  
>       if (aRange.position < 0)
> @@ -276,7 +312,7 @@ WMArray *WMGetSubarrayWithRange(WMArray * array, WMRange 
> aRange)
>  
>  void *WMArrayFirst(WMArray * array, WMArrayIterator * iter)
>  {
> -     if (array->itemCount == 0) {
> +     if (array == NULL || array->itemCount == 0) {
>               *iter = WANotFound;
>               return NULL;
>       } else {
> @@ -287,7 +323,7 @@ void *WMArrayFirst(WMArray * array, WMArrayIterator * 
> iter)
>  
>  void *WMArrayLast(WMArray * array, WMArrayIterator * iter)
>  {
> -     if (array->itemCount == 0) {
> +     if (array == NULL || array->itemCount == 0) {
>               *iter = WANotFound;
>               return NULL;
>       } else {
> @@ -298,6 +334,11 @@ void *WMArrayLast(WMArray * array, WMArrayIterator * 
> iter)
>  
>  void *WMArrayNext(WMArray * array, WMArrayIterator * iter)
>  {
> +     if (array == NULL) {
> +             *iter = WANotFound;
> +             return NULL;
> +     }
> +
>       if (*iter >= 0 && *iter < array->itemCount - 1) {
>               return array->items[++(*iter)];
>       } else {
> @@ -308,6 +349,11 @@ void *WMArrayNext(WMArray * array, WMArrayIterator * 
> iter)
>  
>  void *WMArrayPrevious(WMArray * array, WMArrayIterator * iter)
>  {
> +     if (array == NULL) {
> +             *iter = WANotFound;
> +             return NULL;
> +     }
> +
>       if (*iter > 0 && *iter < array->itemCount) {
>               return array->items[--(*iter)];
>       } else {
> -- 
> 1.8.1.4
> 


-- 
To unsubscribe, send mail to [email protected].

Reply via email to