views:

102

answers:

3

I have been trying out Splint with a C program I recently wrote and trying to understand and remove the warnings it gives. One I understand but can't understand how to remove it comes from the following code snippet:

static MyType_t *findById(const int id)
{
    int i;

    for (i = 0; i < MY_ARR_SIZE; i++) {
            if (my_arr[i].id == NOT_SET) {
                    /* Items are sorted so that items with 
                    NOT_SET as ID are at the end of the array */
                    break;
            }
            if (my_arr[i].id == id) {
                    return &(my_arr[i]);
            }
    }
    return NULL; 
}

Splint isn't happy that the function can return NULL, but in this case it makes perfect sense.

I tried using /@nullwhenfalse@/ but it seems to only work if the function returns true/false and also tried to change the code to use a retVal and tried both /@null@/ and /@relnull@/ in front of the declaration, but these did nothing.

(Just as a side-note, the table is only 20 big atm, so no point in using a clever search algorithm.)

+6  A: 

You should double check the use of /*@null@*/ in front of the declaration.

In the following compilable version of your example, it does remove the warning (using splint 3.1.2):

typedef struct { int id; } MyType_t;
#define NOT_SET -1
#define MY_ARR_SIZE 20
static MyType_t my_arr[MY_ARR_SIZE];

/*@null@*/ static MyType_t *findById(const int id)
{
    int i;
    for (i = 0; i < MY_ARR_SIZE; i++) {
            if (my_arr[i].id == NOT_SET) {
                    /* Items are sorted so that items with 
                    NOT_SET as ID are at the end of the array */
                    break;
            }
            if (my_arr[i].id == id) {
                    return &(my_arr[i]);
            }
    }
    return NULL;
}

int main() {
    (void)findById(10);
    return 0;
}

If you still have a similar warning, could it be about another part of your code ?

Jerome
OK, that works. Thanks!
Makis
+1  A: 

splint -nullret will squash that warning (globally) which may or may not be what you want to do. In some cases, unless you are sure having a type return NULL is correct, you probably want the warning.

I tested Jerome's example, and it did hush the warning for that particular function.

Tim Post
Yes, that would remove the warning, but I don't want to rid of it completely.
Makis
@Makis: Yes, in that case its better to do what Jerome suggests, in fact I've up-voted his answer.
Tim Post
+1  A: 

Whatever you do, I would strongly suggest not embedding splint codes directly into the source, but instead wrap that functionality in a macro.

For example, over on the Parrot project, I have these macros

#  define ARGIN(x)                    /*@in@*/ /*@notnull@*/
#  define ARGIN_NULLOK(x)             /*@in@*/ /*@null@*/
    /* The pointer target must be completely defined before being passed */
    /* to the function. */

#  define ARGOUT(x)                   /*@out@*/ /*@notnull@*/
#  define ARGOUT_NULLOK(x)            /*@out@*/ /*@null@*/
    /* The pointer target will be defined by the function */

And then macros are used so we can use:

void copy_string( ARGOUT(char *target), ARGIN(const char *source ) ) ...

If we want to change how ARGIN() arguments are handled, we change it one place. We can also support multiple notations for multiple tools or compilers.

Andy Lester