Segmentation Error in C code when i try to read the hex file

Question

Im trying to read a hex file, i have partitioned my code into two parts, load file data to buffer and second part access the buffer to read specific region data. The compilation is clean but when i run test, it throws segmentation fault

#include #include #include

char *fip_buffer;
char *emmc_pattern_buffer;

char hex_pattern() { 
    FILE *fileptr;
    char *buffer;
        long filelen;
    int i,j;


    fileptr = fopen("fip.hex", "rb");         
    if( fileptr == NULL ) {
       printf("cannot open file");// exit(1);
     }
    fseek(fileptr, 0, SEEK_END);          
    filelen = ftell(fileptr);            
    rewind(fileptr);                      
    fip_buffer = (char *)malloc((filelen+1)*sizeof(char)); 

    for(i = 0; i < filelen; i++) {
       fread(fip_buffer+i, 1, 1, fileptr); 

    }


    fclose(fileptr); // Close the file
    return(fip_buffer);
}



char hex_pattern_read(int a, int filelen){
char mem[8],mem2[7];
int i,j;
for(i=a;i<filelen;i++){
      mem[j]=fip_buffer[i];
      mem[8]='\0';
      j++;
      if(j==8){strcpy(mem2,mem);j=0; break;
       }
   } 
   emmc_pattern_buffer=mem2;
   return(emmc_pattern_buffer);
}




int main(int argc, char **argv) {
printf("Reading hex file");
int i,j;
hex_pattern();
int len = strlen(fip_buffer);
printf("size of buffer is=%d\n%s\n",len,fip_buffer);
for(i=0; i<2; i++){
  // printf("Entered loop1");
 for(j=0;j<3;j++){
   int temp = (j*8)+(128*i);
    hex_pattern_read(temp,len);
   printf("%s\n",emmc_pattern_buffer);
  }
}
return 0;

}

Your code has multiple issues. Some of them should generate warnings from the compiler. Have you reviewed the compiler output? — kaylum, Jan 12 '20 at 07:00
`printf("%s\n",pattern)`. That causes undefined behaviour because `%s` requires a character buffer (ie, `char *`) but you give it a single `char`. — kaylum, Jan 12 '20 at 07:02
`return(mem2)`. That also causes undefined behaviour as `mem2` is a local array variable and must not be returned to the caller. — kaylum, Jan 12 '20 at 07:03
`return(mem2); fclose(fileptr);`. That results in a resource leak as the `fclose` statement never runs since it is after the `return`. — kaylum, Jan 12 '20 at 07:04
`char mem[8],mem2[7]; mem[8]='\0'; strcpy(mem2,mem)` That causes a buffer overrun as array indices start from 0 and hence `mem[8]` is an invalid access. Also, `strcpy` includes the terminating NUL in the copy and hence having `mem2` smaller than `mem` will result in overrunning `mem2`. — kaylum, Jan 12 '20 at 07:08
I highly recommend a change of learning materials. Whatever you are using is doing you few favours. If this is intended to be C++, [here is a list of generally recognized as good texts and references](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). — user4581301, Jan 12 '20 at 07:28
Thank you kaylum, with the above suggestions my code did work fine. — Bob, Jan 12 '20 at 13:19

RobertS supports Monica Cellio · Answer 1 · 2020-01-12T11:09:49.123

First thing to say, since mem2 is declared by:

char mem[8],mem2[7];

is, that you attempt to return a pointer to char with:

return(mem2);

as opposed to what it is declared in the definition of hex_pattern() as return type; a value of type int:

int hex_pattern(int a) {}

While this on its own may cause the NULL pointer for itself (presume the compiler would let it pass with a warning at least), You even get NULL if you would declare/define hex_pattern() the right way with:

char* hex_pattern (int a){}

mem2 is a pointer to the first element of the local char array of mem2[], but the array of mem2[]is not existing anymore after you will leave hex_pattern() back to the caller, here main().

The array of mem2[]is of storage class auto by default (when you omitting a specific storage class) and an object of that storage class is only alive in the function it was defined/declared. It is determined when leaving the function scope.

So, the pointer points back to NULL, because at the address the pointer is pointing to, is no longer a valid object with a valid value stored.

If you really want to return the whole array of mem2[], which isn´t possible on its own, you can find good alternatives best explained here: Returning an array using C

Also you should never incorporate statements after return, like you did it with:

 return(mem2);
 fclose(fileptr); // Close the file

Simply because of the reason that these statements do not get executed. return is the final statement. Everything what comes thereafter is ignored.

Segmentation Error in C code when i try to read the hex file

1 Answers1