divide by zero in simclist library

Question

In the simclist link list library, I've run into a problem where division by zero occurs in the function list_findpos(). Normally I'm be hesitant to modify a widely used 3rd party library, and there must be good reason why it does what it does (I haven't ruled out the library is used incorrectly). The function is defined as:

/* set tmp to point to element at index posstart in l */
static inline struct list_entry_s *list_findpos(const list_t *restrict l, int posstart) {
    struct list_entry_s *ptr;
    float x;
    int i;

    /* accept 1 slot overflow for fetching head and tail sentinels */
    if (posstart < -1 || posstart > (int)l->numels) return NULL;

    x = (float)(posstart+1) / l->numels;
    if (x <= 0.25) {
        /* first quarter: get to posstart from head */
        for (i = -1, ptr = l->head_sentinel; i < posstart; ptr = ptr->next, i++);
    } else if (x < 0.5) {
        /* second quarter: get to posstart from mid */
        for (i = (l->numels-1)/2, ptr = l->mid; i > posstart; ptr = ptr->prev, i--);
    } else if (x <= 0.75) {
        /* third quarter: get to posstart from mid */
        for (i = (l->numels-1)/2, ptr = l->mid; i < posstart; ptr = ptr->next, i++);
    } else {
        /* fourth quarter: get to posstart from tail */
    for (i = l->numels, ptr = l->tail_sentinel; i > posstart; ptr = ptr->prev, i--);
    }

    return ptr;
}

If l->numels is 0, then the expression on line 9 is a problem. The code otherwise runs ok (without leaks or segfaults), but intel inspector is saying a division by zero occurs on this line. The proposed change is:

/* set tmp to point to element at index posstart in l */
static inline struct list_entry_s *list_findpos(const list_t *restrict l, int posstart) {
    struct list_entry_s *ptr;
    float x;
    int i;

    /* accept 1 slot overflow for fetching head and tail sentinels */
    if (posstart < -1 || posstart > (int)l->numels) return NULL;

    if (l->numels == 0){ //avoid division-by-zero on first element
       ptr = l->head_sentinel;
    }
    else{
       x = (float)(posstart + 1) / l->numels;
       if (x <= 0.25) {
          /* first quarter: get to posstart from head */
          for (i = -1, ptr = l->head_sentinel; i < posstart; ptr = ptr->next, i++);
       }
       else if (x < 0.5) {
          /* second quarter: get to posstart from mid */
          for (i = (l->numels - 1) / 2, ptr = l->mid; i > posstart; ptr = ptr->prev, i--);
       }
       else if (x <= 0.75) {
          /* third quarter: get to posstart from mid */
          for (i = (l->numels - 1) / 2, ptr = l->mid; i < posstart; ptr = ptr->next, i++);
       }
       else {
          /* fourth quarter: get to posstart from tail */
          for (i = l->numels, ptr = l->tail_sentinel; i > posstart; ptr = ptr->prev, i--);
       }
    }
    return ptr;
}

Wondering if anyone can comment on this that has experience using simclist?

EDIT

list_findpos is not an API function; rather, it's an internal function. The sequence of calls leading to the issues are:

/* ... */
list_init(...) 
list_attributes_copy(...)
list_append(...) /* <---- list_findpos() is called here, divide by zero */

It appears to me that `numels` is the number of elements in a list, is that right? An empty list is a common special case in lots of code...just check that on the very first line of the function. If the list is empty, you probably don't want to do anything at all with it, so just take care of that case up front. I'm not sure what all the floating point stuff is about either, but I'll leave that to you. — Lee Daniel Crocker, Jan 16 '15 at 21:48
That's right - `numels` is the number of element declared as `unsigned int numels` — , Jan 16 '15 at 21:58
tweak the function on the line that returns NULL, to also check for numels is 0 as another reason to return NULL — user3629249, Jan 17 '15 at 03:39

score 0 · Accepted Answer · answered Jan 16 '15 at 22:04

0

This function is not part of the library's API; it's totally internal. What the author/maintainer of the library will need to know in order to fix it is what sequence of API calls you made that caused it to fail.

answered Jan 16 '15 at 22:04

Lee Daniel Crocker

12,927
3
29
55

This is actually happening with the sequence of calls in example 2 (ex2.c) distributed with the library. That is, the sequence of calls are: `list_init()`, `list_attributes_copy()`, `list_append()`. The problem is reported when `list_append()` is called. – Jan 16 '15 at 22:41
Seems pretty broken to me. Talk to the author, or find a better library. – Lee Daniel Crocker Jan 16 '15 at 22:43

divide by zero in simclist library

1 Answers1