2

In my company's code we have general get() and set() methods for interop between certain components. However, if I try to run PREfast I get inundated with warnings because PREfast doesn't realize that the get() method initializes the given parameters.

The problem is that since those methods are very general, they don't simply take a parameter (which I could mark with _Out_ or similar, but an array of structs which holds the data as to which data should be returned.

In code (greatly simplified):

typedef struct
{
    int type;
    int* data;
} ARGS;

void get(int count, ARGS* args)
{
    for (int i = 0; i < count; i++)
        *(args[i].data) = 42; // Actually handled by internal methods
}

// Sample Usage
void foo()
{
    int value;
    ARGS args[1];

    args[0].type = 1234;
    args[0].data = &value;

    get(1, args);

    // Do something with value
    // PREfast complains that value is uninitialized (error C6001)
    printf("%d", value);
}

Is there any way to annotate this so PREfast knows that args.data is initialized by get()? Or is this too complex for PREfast to handle?

EDIT: If I use get(1, &args), then the warning goes away. So there is some heuristic in PREfast which can handle this case, but I haven't found out if it is possible to trigger it externally:

void get2(int count, ARGS(* args)[1]) // Needs the size of args, or it won't compile below
{
    for (int i = 0; i < count; i++)
        *(*args)[i].data = 42; // Actually handled by internal methods
}

// Sample Usage
void foo2()
{
    int value;
    ARGS args[1];

    args[0].type = 1234;
    args[0].data = &value;

    get2(1, &args);

    // Do something with value
    printf("%d", value);
}
Daniel Rose
  • 17,233
  • 9
  • 65
  • 88
  • Use a better compiler? I don't get any error here, using clang. – Richard J. Ross III Sep 12 '12 at 12:01
  • @RichardJ.RossIII PREfast is a static analysis tool build on top of the Visual C++ compiler. The normal compiler run gives no warnings, but the additional checks of PREfast give a warning (incorrect in this case). Clang may or may not have these additional checks. – Daniel Rose Sep 12 '12 at 14:12
  • Well then you can use the workaround you mentioned in the EDIT. It doesn't harm the earlier logic, just syntax is a little different. – askmish Oct 20 '12 at 18:06
  • @askmish The problem is that I would need to put the size of args into the get() (or it won't compile). But args could be an array of one or two or ten (that is why there is a parameter `count`). So that doesn't work, unfortunately. – Daniel Rose Oct 20 '12 at 18:08
  • Did you try `**args` instead of `*args[1]`? – askmish Oct 20 '12 at 18:11
  • @askmish error C2664: 'get2' : cannot convert parameter 2 from 'ARGS (*)[1]' to 'ARGS **' Note that a pointer to an array is usually a mistake; see http://c-faq.com/aryptr/ptrtoarray.html But if I use &args[0], the warning is there again. – Daniel Rose Oct 20 '12 at 18:15
  • @Daniel: I was just trying to avert the static array thing. But, There seems no other way I could think of now. You have to use `suppress` or call the vendor for support or wait for a bug fix. – askmish Oct 20 '12 at 18:20

1 Answers1

1

This should fix the warning.

void foo()
{
   int value=0;
  ...
}

Note that get() will be called in runtime only. Since, PREfast is a static analysis tool, it might report that the value is uninitialized. Nevertheless, initializing a variable before use is always a best practice in C.

Another way would be to use the PREfast suppress as below:

void foo()
{
    int value;
    ARGS args[1];

    args[0].type = 1234;
    args[0].data = &value;

    get(1, args);

    // Do something with value
    // PREfast complains that value is uninitialized (error C6001)
    #pragma prefast(suppress:C6001 , "PREfast noise: the variable value will be initialized by get method in a line above")
    printf("%d", value);
}

It suppresses the warnings in the next line after the suppress statement.

Also, do add the following code in you header files(or source files) just before using the pragma prefast in your code:

#ifndef _PREFAST_
#pragma warning(disable:4068)
#endif

to avoid 4068 warning to be flagged. NOTE: pragma prefast is an extension to the PREfast AST compiler only and may not be supported by other compilers.

askmish
  • 6,464
  • 23
  • 42
  • Unfortunately, your ideas won't work. I have tens of thousands of variables initialized by get(). So manually initializing all those variables or suppressing at the point of use isn't feasible. The only places where I could change something would be (a) in get() or (b) at the statement `args[0].data = &value;` (since that is actually a macro). However, value may also be an double, pointer, or even a struct, so a simple `value = 0;` won't work. – Daniel Rose Oct 20 '12 at 17:14
  • Note that if I change the definition of get() slightly to pass &args, then the warning goes away! But since args is an array, that doesn't compile in my application's code, since args is not the same length everywhere. Basically what I want is to tell PREfast "treat everything in args as initialized", just as PREfast does when you pass an address of a variable. – Daniel Rose Oct 20 '12 at 17:18
  • @Daniel:From you described, it seems like a PREfast analysis bug to me. Did you ask the PREfast support or their forums, about this? Did you try the PREfast pragma `suppress` I have mentioned in the answer? If you are sure that you are writing a proper code, its absolutely ok to suppress those errors. Software cannot be 100% right always. – askmish Oct 20 '12 at 17:28
  • I asked the same question in the PREfast forum and the general forum, but got no useful answer (besides completely suppressing C6001). I haven't tried prefast suppress, but as I stated there are tens of thousands of places in the code. So if I have to put it at the point of use of value, then it won't help. I need code at the point of assigning value to args.data or in get(). – Daniel Rose Oct 20 '12 at 17:38
  • @Daniel: In your previous comment you were talking about a workaround you found right? Could you post that in the question edit? I guess that will be the only way to do it...If you do not want to use `suppress`. – askmish Oct 20 '12 at 17:43
  • @DanielRose: There is a way to `suppress` at file level but that would be not preferable, since it may hide warnings at places which actually needs attention and not due to this bug. – askmish Oct 20 '12 at 17:48