function returning a struct

Discussion in 'Mac Programming' started by dougphd, Aug 14, 2016.

  1. dougphd macrumors member

    Joined:
    Apr 28, 2016
    #1
    I wrote a function that returned a structure. It wouldn't work until I fixed the warning that said it hadn't been initialized. What is that all about?
     
  2. dylanryan macrumors member

    Joined:
    Aug 21, 2011
    #2
    Post your code. Did it return the struct by value, or by reference (i.e. as a pointer)?
     
  3. dougphd thread starter macrumors member

    Joined:
    Apr 28, 2016
    #3
    I'm not really interested in why my code especially but in a broader discussion but here's the code.
    Code:
    struct etf_data
    
    {
    
        char date[days_of_data][14];
    
        //char name[20];
    
        //float sauce;
    
        float open[days_of_data];
    
        //int signal;
    
        int index[days_of_data];
    
        //float value;
    
    } ;
    
    
    
    structetf_data read_data(FILE *data, int *num)
    
    {
    
        char line[200];
    
       
    
        structetf_data DATA = {};
    
        num = (int*) calloc(1,sizeof(int));
    
      //read header
    
        fgets(line,sizeof(line),data);
    
      //  printf("%s\n", line);
    
    
        *num = 0;
    
        while ((fgets(line,sizeof(line),data)) != NULL)
    
        {
    
          // printf("%d %s",*num, line);
    
            strtok(line,","); //symbol
    
            strtok(NULL,","); //w
    
            strcpy(DATA.date[*num],strtok(NULL,",")); //date
    
            strtok(NULL,","); //000000
    
            DATA.open[*num]= atof(strtok(NULL,","));
    
            DATA.index[*num] = *num;
    
        //    printf(" %f, %s\n", DATA.open[*num], DATA.date[*num] );
    
            (*num)++;
    
        }
    
    return (DATA);
    
    };
    
     
  4. dylanryan macrumors member

    Joined:
    Aug 21, 2011
    #4
    That code, once I add some definitions, compiles just fine. I'm assuming the code with the issues didn't have the ={} bit and just declared
    struct etf_data DATA ?

    If so, the compiler warning is because the struct contains a bunch of arrays of a specific predefined size, and the loop that fills them never references that size at all. If your loop ends early (say that days_of_data is 100 and your file only has 10 items in it), then all the subsequent entries in the array will be uninitialized memory with garbage values in it. The compiler is warning you that it cannot prove to itself that the code will always initialize every value in the struct, and therefore if you can't prove it will, you might have issues. By adding the empty initializer, you set all the memory to a default value (for these types, probably 0), and all is well. (also, if days_of_data is 10 but your file actually has 100 lines, then you will currently happily over-run your buffers and write data to memory you haven't allocated. The initializer does nothing to stop that).

    The basic issue: Because your loop is basically "while there are still lines in the file", and it could be run on any old file (even if you hardcode the name, the file can be changed), the compiler cannot possibly prove that for every possible situation, every element in those arrays will be initialized. If it can't prove it, it emits a warning because that means you need to be aware of it. If you can prove it, then you can give it the information it needs to prove that is the case (either by explicitly initializing things, or using a loop that always iterates days_of_data times, each one initializing a different element). This has nothing to do with the struct and everything to do with the arrays.

    As an aside: This would make a whole lot more sense as an array of structs, where each struct just has a char[14], float, and int in it, rather than 1 struct with parallel arrays of each of those. The error would be the same, but the code would be a lot easier to follow, I think.
    --- Post Merged, Aug 14, 2016 ---
    Actually, I take part of that back: the struct does matter because it is when you make a copy of the struct (in this case, the return value of the function is copied when you return it) that the compiler complains about uninitialized members. So, the warning is only because a struct is involved.

    Here is an example program that demonstrates the same issue, but asks the user for data on the command line. Try entering 3 numbers as requested. Then try it again entering 1, or 5 numbers, and see what happens. You can then comment out "struct data d;" and uncomment the subsequent line to see what happens when it is fully initialized).

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    struct data {
        int buffer[5];
        int data[3];
        int buffer2[5];
    };
    
    void foo(void);
    void doIt(void);
    
    void foo(void) {
        // just putting some random data on the stack. This doesn't do anything at all
        volatile int x[2048];
        for(int i=0;i<2048;++i)
            x[i]=i*i;
    }
    
    void doIt(void) {
        struct data d;
        // struct data d={.buffer={42},.data={0},.buffer2={25}};
    
        char buf[1024];
        char* tok;
    
    
        // initialize those two buffers so we can tell when we overrun
        // You can delete this if you switch to the second way of declaring d above
        for(int i=0;i<5;++i) {
            d.buffer[i]=42;
            d.buffer2[i]=24;
        }
    
        printf("Please enter 3 integers, separated by spaces, then press enter.\n");
        if(fgets(buf,1024,stdin)==NULL) {
            printf("fgets returned 0\n");
            exit(1);
        }
    
        int ct=0;
        while((tok=strtok(ct?NULL:buf," "))!=NULL) {
            d.data[ct++]=atoi(tok);
        }
    
        struct data dat=d; // copy the entire thing, this is what causes the warning
    
        printf("DATA:\n");
        for(int i=0;i<3;++i) {
            printf("  %d: %d\n",i,dat.data[i]);
        }
    
        printf("PRE BUFFER:\n");
        for(int i=0;i<5;++i) {
            printf("  %d: %d\n",i,dat.buffer[i]);
        }
    
        printf("POST BUFFER:\n");
        for(int i=0;i<5;++i) {
            printf("  %d: %d\n",i,dat.buffer2[i]);
        }
    }
    int main() {
        foo();
        doIt();
    
        return 0;
    }
    
     
  5. dougphd thread starter macrumors member

    Joined:
    Apr 28, 2016
    #5
    thanks. This will take a while to digest. However, on a more basic level isn't a warning just that? Why did the code fail if it was just a warning?
     
  6. dylanryan macrumors member

    Joined:
    Aug 21, 2011
    #6
    You haven't said how the code failed, so I can't be sure.

    That said, I have a guess. My guess is that you are not fully filling in all the arrays when reading from the file (perhaps your arrays are 1 element too long, for example. Or maybe 100 too long, I have no way of knowing). I would guess that when you subsequently process the data, if you hit an array where all the values are 0, your processing basically does nothing (i.e. it loops 0 times, for example), which is safe. So, when you initialize the arrays, any unused entries are all 0s and your subsequent processing "works" by doing nothing. However, if you don't initialize them, then your subsequent processing hits garbage data and maybe tries to loop a billion times, or 53 times, or whatever, and it fails on the junk data it gets.

    If that is the case, initializing the array didn't actually fix anything, it just forced the code into a safe mode of failure (trying to process uninitialized entries is a failure state, but if they are zeroed, it is harmless), whereas without the data being initialized, you have a potentially unsafe mode of failure.

    That is just my guess. There is nowhere near enough information here to do much better than that. How exactly did it fail (exact error messages are helpful). Was it always the same failure or did it change each time you ran it (all else, like file contents, held equal)? Are you absolutely 100% certain the arrays are always filled in, no matter what happens and what is in the files?

    This is why we always ask people to post all their code and relevant data files, and give exact error messages. Without knowing exactly what you did, we have no way to know what happened (and if you are running a different OS than I am, I might not be able to reproduce it because things change OS to OS, let alone CPU architecture to CPU architecture, so knowing what errors you saw lets us debug even if we can't reproduce).


    Also, just because something is "just a warning" doesn't mean the code won't fail to work as expected.

    Try this out:

    Code:
    #include <stdio.h>
    
    int main() {
        int x=-1;
        unsigned int max=x;
    
        for(unsigned int i=0;i<max;++i) {
            printf("%d\n",i);
        }
        return 0;
    }
    
    ( You will probably want to ^C that )

    That has "just a warning" but it almost certainly "doesn't work" by any reasonable definition of working. It is best to treat warnings as errors that have a high enough false-positive rate that they can't be made errors. Almost every time they occur, something has gone wrong, though a few cases are things where the compiler is actually wrong and the code is fine. But generally speaking, never assume that is the case. Unless you can prove it is the case, treat the warning as an error.
     
  7. robvas macrumors 68020

    Joined:
    Mar 29, 2009
    Location:
    USA
    #7

Share This Page