Bugs item #3042655, was opened at 2010-08-10 10:56
Message generated for change (Comment added) made by wsalivar
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3042655&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: hang
Status: Open
Resolution: Invalid
Priority: 5
Private: No
Submitted By: Bill Salivar (wsalivar)
Assigned to: Daniel Stenberg (bagder)
Summary: Infinite loop condition in glob_cleanup() (urlglob.c)

Initial Comment:
A run against version 7.19.4 of libCurl using Coverity Prevent has found an infinite loop condition. I have checked the latest version of libCurl (7.21.0) and the code is the same. Any idea what the correct loop logic should be?

374 void glob_cleanup(URLGlob* glob)
375 {
376 size_t i;
377 int elem;
378
Event non_progress_update: Update "--i" makes no progress toward falsifying loop condition "i < glob->size"
Event loop_top: Top of loop.
Event loop_bottom: Bottom of loop.
Event loop_condition: "i < glob->size" must remain true for the loop to continue.
379 for (i = glob->size - 1; i < glob->size; --i) {
380 if (!(i & 1)) { /* even indexes contain literals */
381 free(glob->literal[i/2]);
382 }
383 else { /* odd indexes contain sets or ranges */
384 if (glob->pattern[i/2].type == UPTSet) {
385 for (elem = glob->pattern[i/2].content.Set.size - 1;
386 elem >= 0;
387 --elem) {
388 free(glob->pattern[i/2].content.Set.elements[elem]);
389 }
390 free(glob->pattern[i/2].content.Set.elements);
391 }
392 }
393 }
394 free(glob->glob_buffer);
395 free(glob);
396 }

----------------------------------------------------------------------

>Comment By: Bill Salivar (wsalivar)
Date: 2010-08-10 13:07

Message:
Thanks for clarifying the intent of the code. You can close this and I can
report the mis-diagnosis with Coverity.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2010-08-10 11:16

Message:
I claim this rather points to a flaw in your checker tool.

--i does indeed make "progress toward falsifying loop condition", because
'i' is a variable of type size_t which is an unsigned variable, so it will
wrap from 0 to a very large value and then the loop condition will no
longer equal true. Also, if this report were correct, how come we've been
able to use this exact code since July 2004 without any hang?

Changing the loop condition would probably force us to use 'i' signed,
check for i >= 0 in the loop and have added typecasts for when we assign i
from glob->size which is a size_t type.

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3042655&group_id=976
Received on 2010-08-10