function returns address out of bounds

[asilter]

Hi,
I'm trying to assign variables where vpSec0 points.

void * vpSec0 = NULL;
CreateHVFESection0(vpSec0);

CreateHVFESection0 function is below.

void CreateHVFESection0(void * vpSec0)
{
        int hSec0;      
        size_t * nbytes = (size_t *) malloc(sizeof(size_t));
        hSec0 = bitio_o_open(); 
 
        /* 'B','U','F','R' */
        bitio_o_append(hSec0,66,8);
        bitio_o_append(hSec0,85,8);
        bitio_o_append(hSec0,70,8);
        bitio_o_append(hSec0,82,8);
 
        /* Total length of BUFR message in bytes */
        bitio_o_append(hSec0,0,24);
 
        /* BUFR Edition Number = 4 */
        bitio_o_append(hSec0,4,8);
        vpSec0 = bitio_o_close(hSec0, nbytes);
 
        free(nbytes);
        nbytes = NULL;
 
 
}

CreateHVFESection0 uses some other functions. But I think the problem is here:

vpSec0 = bitio_o_close(hSec0, nbytes);

because i run the program with gdb debugger. Before coming here, vpSec0 is 0x0, means NULL. Everything is normal to here.

void *bitio_o_close (handle, nbytes)
int handle;
size_t *nbytes;
/* This function closes a output-bitstream identified by HANDLE and returns
   a pointer to the memory-area holding the bit-stream.
   parameters:
   HANDLE:  Bit-stream-handle
   NBYTES:  number of bytes in the bitstream.
   The funcion returns a pointer to the memory-area holding the bit-stream or
   NULL if an invalid handle was specified. The memory area must be freed by
   the calling function.
*/
{
  if (!bios[handle].used) return NULL;
/******* Fill up the last byte with 0-bits */
  while (bios[handle].nbits % 8 != 0) bitio_o_append (handle, 0, 1);
  *nbytes = (size_t) ((bios[handle].nbits - 1) / 8 + 1);
  bios[handle].used = 0;
  return (void *) bios[handle].buf;
}
void *bitio_o_close (handle, nbytes)
int handle;
size_t *nbytes;
/* This function closes a output-bitstream identified by HANDLE and returns
   a pointer to the memory-area holding the bit-stream.
   parameters:
   HANDLE:  Bit-stream-handle
   NBYTES:  number of bytes in the bitstream.
   The funcion returns a pointer to the memory-area holding the bit-stream or
   NULL if an invalid handle was specified. The memory area must be freed by
   the calling function.
*/
{
  if (!bios[handle].used) return NULL;
/******* Fill up the last byte with 0-bits */
  while (bios[handle].nbits % 8 != 0) bitio_o_append (handle, 0, 1);
  *nbytes = (size_t) ((bios[handle].nbits - 1) / 8 + 1);
  bios[handle].used = 0;
  return (void *) bios[handle].buf;
}

when i step into bitio_o_close function, before

return (void *) bios[handle].buf

, the address of bios[handle].buf is 0x2a99700930. So we expect that after returning, vpSec0's address will also be 0x2a99700930. But after returning when i print vpSec0, it's address seems 0xffffffff99700930, and this is out of bounds which falls me in Segmentation faults further in my program.
Please help.
Thanx.

[Ancient Dragon]

some questions:
Function CreateHVFESection0().

1. Why is vpSec0 a parameter? It could just as easily be declared locally in the function since it is never used for anything.

2. Why do you bother to malloc nbytes since sizeof(size_t) is only 4 bytes in 32-bit compilers ? Just declare it normally and used the address operator & when used near the bottom of the function.

3. Your function has memory leak because it does not free the memory location returned by bitio_o_close(), as indicated that it needs to do in the function description that follows. You need to add another line free(vpSec0); 4. Since we know nothing about function bitio_o_append() could you please post its function prototype like you did with bitio_o_close()? Maybe the parameters you are sending it are incorrect of wrong type.

I think something has screwed up the stack before calling bitio_o_close(). I would start by commenting out all the calls to bitio_o_append() then rerun and see if the bad pointer problem is corrected. If not then most likely the problem is somewhere else.

[asilter]

some questions:
Function CreateHVFESection0().

1. Why is vpSec0 a parameter? It could just as easily be declared locally in the function since it is never used for anything.

I have 6 sections to be created. Then I'm going to create a stream consisting all 6 sections. So, it's going to be used.

2. Why do you bother to malloc nbytes since sizeof(size_t) is only 4 bytes in 32-bit compilers ? Just declare it normally and used the address operator & when used near the bottom of the function.

System is 64 bits. So this malloc is the worst I think:)
Because the bitio_o_close function wants a pointer of type size_t. Can I declare is as

size_t nbytes = NULL

then when sending &nbytes is ok you say I think. What's the difference between sending malloced pointer address and sending variable address? Then, when do we need mallocs?

3. Your function has memory leak because it does not free the memory location returned by bitio_o_close(), as indicated that it needs to do in the function description that follows. You need to add another line free(vpSec0);

Later I'm going to use vpSec0 to bring all sections together into another void pointer.

4. Since we know nothing about function bitio_o_append() could you please post its function prototype like you did with bitio_o_close()? Maybe the parameters you are sending it are incorrect of wrong type.

long bitio_o_append (handle, val, nbits)
int handle;
unsigned long val;
int nbits;
/* This function appends a value to a bitstream.
   parameters:
   HANDLE:  Indicates the bitstream for appending.
   VAL:     Value to be output.
   NBITS:   Number of bits of VAL to be output to the stream. Note that NBITS
            muste be less that sizeof (LONG)
   The return-value is the bit-position of the value in the bit-stream, or -1
   on a fault.
*/
{
/******* Check if bitstream is allready initialized and number of bits does not
         exceed sizeof (unsigned long). */
  assert (bios[handle].used);
  assert (sizeof (unsigned long) * 8 >= nbits);
/******* check if there is enough memory to store the new value. Reallocate
         the memory-block if not */
  if ((bios[handle].nbits + nbits) / 8 + 1 > (long) bios[handle].size) {
    bios[handle].buf = realloc (bios[handle].buf, bios[handle].size + INCSIZE);
    if (bios[handle].buf == NULL) return 0;
        memset (bios[handle].buf + bios[handle].size, 0, INCSIZE);
    bios[handle].size += INCSIZE;
  }
/******* output data to bitstream */
  bitio_o_outp (handle, val, nbits, bios[handle].nbits);
  bios[handle].nbits += nbits;
  return bios[handle].nbits;
}
 
void bitio_o_outp (handle, val, nbits, bitpos)
int handle;
unsigned long val;
int nbits;
long bitpos;
/* This function outputs a value to a specified position of a bitstream
   parameters:
   HANDLE:  Indicates the bitstream for output.
   VAL:     Value to be output.
   NBITS:   Number of bits of VAL to be output to the stream. Note that NBITS
            must be less then sizeof (LONG)
   BITPOS:  bitposition of the value in the bitstream.
*/
{
  int i, bit, bitval;
  size_t byte;
  char *pc, c;
/******* Check if bitstream is allready initialized and number of bits does not
         exceed sizeof (unsigned long). */
  assert (bios[handle].used);
  assert (sizeof (unsigned long) * 8 >= nbits);
  for (i = nbits - 1; i >= 0; i --) {
/******* Get bit-value */
    bitval = (int) (val >> i) & 1;
/******* calculate bit- and byte-number for output */
    /*byte = (int) (bitpos / 8);
    bit  = (int) (bitpos % 8);*/
    byte = (int) (bitpos / 8);
    bit  = (int) (bitpos % 8);
    bit  = 7 - bit;
/******* set bit-value to output stream */
    pc = bios[handle].buf + byte;
    if (bitval) {
      c = (char) (1 << bit);
      *pc |= c;
    }
    else {
      c = (char) (1 << bit);
      c ^= 0xff;
      *pc &= c;
    }
    bitpos ++;
  }
}

I think something has screwed up the stack before calling bitio_o_close(). I would start by commenting out all the calls to bitio_o_append() then rerun and see if the bad pointer problem is corrected. If not then most likely the problem is somewhere else.

trying.

[Salem]

> CreateHVFESection0(vpSec0);
Following the call, vpSec0 will still be NULL.

> vpSec0 = bitio_o_close(hSec0, nbytes);
This does NOT update the variable passed as a parameter in the caller. It only updates the formal parameter copy of that variable.
If you're really trying to return this result to the caller, then you need a different approach.

[Ancient Dragon]

>>What's the difference between sending malloced pointer address and sending variable address?
Efficiency mainly. malloc() requires a lot of time and some overhead.

>> Then, when do we need mallocs?
When you don't know the size of something at compile time. For example if you need an array of structures but don't know how many until the program is executed. Also when the size of an object is just too big to be allocated on the stack.

>>size_t nbytes = NULL
No, NULL is used for pointers, not integers. size_t nbytes = 0; >> bitio_o_append(hSec0,0,24);
That line is wrong, the 3d parameter should be sizeof(LONG). Never assume a LONG is a specific size -- always use the sizeof operator to make your program more portable and correct. And the sizeof(LONG) is not 24 in either 32-bit or 64-bit systems, so I don't know where you get the value of 24 from.

[asilter]

> CreateHVFESection0(vpSec0);
Following the call, vpSec0 will still be NULL.

> vpSec0 = bitio_o_close(hSec0, nbytes);
This does NOT update the variable passed as a parameter in the caller. It only updates the formal parameter copy of that variable.
If you're really trying to return this result to the caller, then you need a different approach.

i changed the code to:

void * vpSec0 = NULL;
CreateHVFESection0(&vpSec0);

and the function like this:

void CreateHVFESection0(void ** vpSec00)
{
        size_t nbytes = 0;
        int hSec0 = 0;
 
        hSec0 = bitio_o_open();
        bitio_o_append(hSec0,66,8);
 
        *vpSec00 = bitio_o_close(hSec0, &nbytes);
 
}

and try. still the address is out of bounds like 0xffffffff99700910
. in the debug; just before returning in bitio_o_close, the address was 0x2a99700910. I don't understand the change of address in one line of execution.

[Ancient Dragon]

did you check the return value of bitio_o_open() ? Maybe its returning NULL (or 0) ? If that is ok, then my bet is the problem is somewhere else. Does bitio_o_append() work correctly? How about the function that calls CreateHVFESection0().

Does your program use structures? Are they declared correctly in all program units ? Also check for array bounds errors and buffer overflows, which will both cause wierd problems in unexpected places.

[asilter]

>> bitio_o_append(hSec0,1500,24);
That line is wrong, the 3d parameter should be sizeof(LONG). Never assume a LONG is a specific size -- always use the sizeof operator to make your program more portable and correct. And the sizeof(LONG) is not 24 in either 32-bit or 64-bit systems, so I don't know where you get the value of 24 from.

if you look at NBITS comment in bitio_o_append function, it says it can be less than sizeof(LONG) (8 bytes , 64 bits in my system). NBITS is the number of bits which VAL value would be stored as stream. VAL value can be of an argument valued, say 1500. Store it in 3 bytes I say.

But i changed my code to this:

void CreateHVFESection0(void ** vpSec00)
{
        size_t nbytes = 0;
        int hSec0 = 0;
 
        hSec0 = bitio_o_open();
        bitio_o_append(hSec0,66,8);
 
        *vpSec00 = (void *) bitio_o_close(hSec0, &nbytes);
 
}

again the returning address is out of bounds. within gdb debugger just before return statement, i do :
print bios[handle].buf
and gdb debuger prints:

$2 = 0x2a99700910 "B"

after returning while i was in CreateHVFESection0 function,i say:
print *vpSec00
and gdb debuger prints:

$3 = (void *) 0xffffffff99700910

I don't understand the change in one line of execution.

[asilter]

did you check the return value of bitio_o_open() ? Maybe its returning NULL (or 0) ? If that is ok, then my bet is the problem is somewhere else. Does bitio_o_append() work correctly? How about the function that calls CreateHVFESection0().

Does your program use structures? Are they declared correctly in all program units ? Also check for array bounds errors and buffer overflows, which will both cause wierd problems in unexpected places.

Yes there are structures.

#define MAXSIZE 64
 
typedef struct radar_data
{
char * sRadarName;
long size;
DataDate * ddYearMonthDay;
DataTime * dtHourMinuteSecond;
LLBulkData * llBdContent;
short int nRdbType;
short int nRdbSubtype;
}RadarData; 
 
/* data stores 64 bytes of streams. */
typedef struct bulk_data
{
char * data;
struct bulk_data * next;
}BulkData;                                                                               
 
/* This is root of linked list */
typedef struct ll_bulk_data
{
long size;
BulkData * bdContent;
}LLBulkData;

If there is a problem, it should be below.
I create a RadarData like this:

RadarData * GetRadarData(char * sFileName)
{
        short int nRadarSubtypeInFile;
        short int nRadarTypeInFile;
        /* Radar Data Structure */
        RadarData * rd =  (RadarData *) malloc(sizeof(RadarData));
        /* Radarin adi */
        char * sRadarName =  (char *) malloc(4);
        /* Radar tarih,saat bilgisi. */
        char * sYear = (char *) malloc(2);
        char * sMonth = (char *) malloc(2);
        char * sDay = (char *) malloc(2);
        char * sHour = (char *) malloc(2);
        char * sMinute = (char *) malloc(2);
        char * sSecond = (char *) malloc(2);
        rd->sRadarName = (char *) malloc(4);
        rd->ddYearMonthDay = (DataDate*) malloc(sizeof(DataDate));
        rd->dtHourMinuteSecond = (DataTime*) malloc(sizeof(DataTime));
        rd->llBdContent = (LLBulkData *) malloc(sizeof(LLBulkData));
        rd->llBdContent->bdContent = (BulkData *) malloc(sizeof(BulkData));
        rd->llBdContent->bdContent->data = (char *) malloc(MAXSIZE);
        /* Radar Name */
        sRadarName = strncpy(sRadarName,sFileName,3);
        sYear = strncpy(sYear,sFileName+3,2);
        sMonth = strncpy(sMonth,sFileName+5,2);
        sDay = strncpy(sDay,sFileName+7,2);
        sHour = strncpy(sHour,sFileName+9,2);
        sMinute = strncpy(sMinute,sFileName+11,2);
        sSecond = strncpy(sSecond,sFileName+13,2);
/* ASSIGNMENT TO STRUCTURE */
        rd->sRadarName = strcpy(rd->sRadarName,sRadarName);
        rd->sRadarName = strcat(rd->sRadarName,"1");
        rd->ddYearMonthDay->nYear = atoi(sYear);
        rd->ddYearMonthDay->nMonth = atoi(sMonth);
        rd->ddYearMonthDay->nDay = atoi(sDay);
        rd->dtHourMinuteSecond->nHour = atoi(sHour);
        rd->dtHourMinuteSecond->nMinute = atoi(sMinute);
        rd->dtHourMinuteSecond->nSecond = atoi(sSecond);
        nRadarSubtypeInFile = GetRdbSubtype(rd->sRadarName);
        rd->nRdbSubtype = nRadarSubtypeInFile;
        nRadarTypeInFile = GetRdbType(rd->sRadarName);
        rd->nRdbType = nRadarTypeInFile;
        free(sRadarName);
        sRadarName = NULL;
        free(sYear);
        sYear = NULL;
        free(sMonth);
        sMonth = NULL;
        free(sDay);
        sDay = NULL;
        free(sHour);
        sHour = NULL;
        free(sMinute);
        sMinute = NULL;
        free(sSecond);
        sSecond = NULL;
        /* Bulk Data buradan itibaren okunup, long size, char data[] bilgisi giriliyor. */
        SetBulkData(rd,sFileName);
        return rd;
}
 
 
void SetBulkData(RadarData * rd, char * sFileName)
{
        int nFileSize;
        LLBulkData * llBulkDataDummy = NULL;
        BulkData * bdDummy = NULL;
        /* 3 byte for section4 size (3 byte max integerdan 4 byte cikar) */
        /* int nMaxByteCount = 16777211;  */
        int nMaxByteCount = MAXSIZE * 1000000;
        double dBulkDataCount;
        int nLastBulkDataSize;
        int nBulkDataCount = 0;
        int nDummyFileSize = 0;
        /* Tam Dosya yolu */
        char * sRawFilePath =  (char *) malloc(120);
        /* fopen source */
        char * sFile =  (char *) malloc(200);
        int i;
        FILE * DataFile = NULL;
        char * caFileContent = (char *) malloc(nMaxByteCount);
        char * sDummyFileContentFirst = (char *) malloc(MAXSIZE);
        char * sDummyFileContent = NULL;
        int nAdditiveBulk = 0;
        sRawFilePath = strcpy(sRawFilePath,"data/");
        sFile = strcat(sRawFilePath,sFileName);
        DataFile = fopen(sFile,"rb");
        if(DataFile == NULL)
                printf("DataFile NULL\n");
        printf("(Radar.c) nMaxByteCount = %d\n",nMaxByteCount);
printf("(Radar.c) MAXSIZE = %d\n",MAXSIZE);
        nFileSize = fread(caFileContent,sizeof(char),nMaxByteCount,DataFile);
        nDummyFileSize = nFileSize;
        fclose(DataFile);
        printf("nFileSize = %d\n",nFileSize);
        dBulkDataCount = nDummyFileSize / MAXSIZE;
        printf("(Radar.c) dBulkDataCount = %2.2f\n",dBulkDataCount);
        nBulkDataCount = (int) dBulkDataCount;
        if(nDummyFileSize % MAXSIZE > 0)
                nBulkDataCount ++ ;
        printf("(Radar.c) nBulkDataCount = %d\n",nBulkDataCount);
        nLastBulkDataSize = nMaxByteCount % MAXSIZE;
        printf("(Radar.c) nLastBulkDataSize = %d\n",nLastBulkDataSize);
        printf("nBulkDataCount = %d\n", nBulkDataCount);
        for(i=0; i<nBulkDataCount; i++)
        {
                if(i==0)
                {
                        llBulkDataDummy = (LLBulkData *)CreateBulkDataLinkedList();
                        memcpy(sDummyFileContentFirst,caFileContent,MAXSIZE);
                        bdDummy = (BulkData *) CreateNewBulkDataNode();
                        memcpy(bdDummy->data,sDummyFileContentFirst,MAXSIZE);
                        AddBulkDataToLinkedList(llBulkDataDummy,bdDummy);
                        nDummyFileSize = nDummyFileSize - MAXSIZE;
                        free(sDummyFileContentFirst);
                        sDummyFileContentFirst = NULL;
}
                else
                {
                        sDummyFileContent = (char *) EraseCopiedBytes(caFileContent+i*MAXSIZE, nDummyFileSize);
                        if(sDummyFileContent == NULL)
                                break;
                        bdDummy = (BulkData *) CreateNewBulkDataNode();
                        memcpy(bdDummy->data,sDummyFileContent,MAXSIZE);
                        AddBulkDataToLinkedList(llBulkDataDummy,bdDummy);
                        nDummyFileSize = nDummyFileSize - MAXSIZE;
                        free(sDummyFileContent);
                        sDummyFileContent = NULL;
                        sDummyFileContent = realloc(sDummyFileContent,nDummyFileSize-MAXSIZE);
                }
        }
        nAdditiveBulk = nDummyFileSize;
        printf("nAdditiveBulk = %d\n",nAdditiveBulk);
        if(nLastBulkDataSize > 0)
        {
                bdDummy = CreateNewBulkDataNode();
                memcpy(bdDummy->data,sDummyFileContent,nLastBulkDataSize);
                realloc(bdDummy->data,nAdditiveBulk);
        }
        free(sFile);
        sFile = NULL;
        free(caFileContent);
        caFileContent = NULL;
        PrintLinkedList(llBulkDataDummy);
}
 
 
char * EraseCopiedBytes(char * caFileContent, int nSize)
{
        int i;
        int nNewSize = nSize;
        if(nNewSize == 0)
                return NULL;
        char * caFileContentNew = (char *) malloc(nNewSize);
        memcpy(caFileContentNew,caFileContent,nNewSize);
        return caFileContentNew;
}
 
BulkData * CreateNewBulkDataNode()
{
        BulkData * bdDummy = (BulkData *) malloc(sizeof(BulkData));
        bdDummy->data = (char *) malloc(MAXSIZE);
        bdDummy->next = NULL;
        return bdDummy;
}
 
LLBulkData * CreateBulkDataLinkedList()
{
        LLBulkData * llBdDummy = (LLBulkData *) malloc(sizeof(LLBulkData));
        llBdDummy->bdContent = NULL;
        llBdDummy->size = 0;
        return llBdDummy;
}
 
 
void AddBulkDataToLinkedList(LLBulkData * llBulk, BulkData * bdNode)
{
        int i;
        int nCounter = 0;
        BulkData * bdDummy = NULL;
        BulkData * bdWaitingToBeCopied = (BulkData *) malloc(sizeof(BulkData));
        memcpy(bdWaitingToBeCopied,bdNode,sizeof(bdNode));
        printf("(Radar.c)(AddBulkDataToLinkedList) evet1\n");
        if(llBulk->size == 0)
        {
                llBulk->bdContent = bdWaitingToBeCopied;
        }
        else
        {
                for(bdDummy = llBulk->bdContent; bdDummy->next != NULL; bdDummy = bdDummy->next)
                {
                        nCounter ++;
                }
                bdDummy->next = bdWaitingToBeCopied;
        }
        printf("(Radar.c)(AddBulkDataToLinkedList) evet2\n");
        llBulk->size++;
        free(bdNode);
        bdNode = NULL;
}
 
void PrintLinkedList(LLBulkData * llBulk)
{
        BulkData * bdDummy = NULL;
        int i;
        for(bdDummy = llBulk->bdContent; bdDummy != NULL; bdDummy = bdDummy->next)
        {
                for(i=0;i<MAXSIZE;i++)
                        printf("%d,",bdDummy->data[i]);
                printf("\n...\n");
        }
 
}

[Salem]

> void *bitio_o_close (handle, nbytes)
> int handle;
> size_t *nbytes;
So why are you using very old style K&R C here, and not ANSI-C with proper function definitions (and prototypes).

Talking of prototypes, does the caller really know that the function returns a void*, or are just casting everything to keep the compiler from telling you something important?

[Ancient Dragon]

Boy! you really love to use malloc() liberally don't you, even when its more efficient not to. For example:

#define RADARNAMESIZE   4
typedef struct radar_data
{
char  sRadarName[RADARNAMESIZE];
long size;
DataDate  ddYearMonthDay;
DataTime  dtHourMinuteSecond;
LLBulkData * llBdContent;
short int nRdbType;
short int nRdbSubtype;
}RadarData;

There are hundreds of buffer overruns in GetRadarData() which is probably causing your original complaint. When calling strcpy() or strncop() the destination buffer must be at least 1 byte larger than the number of characters you want to copy. For example, you want to put 2 characters in the Year array, so do this:

char sYear[3]; // enough room for 2 characters plus null terminator

strncpy(sYear,sFileName+3,2);

>> sYear = strncpy(sYear,sFileName+3,2);
The line above may destroy the pointer sYear that was previously allocated if strncpy() fails. See previous code I posted for example of how to correct it. You have to correct all the other small char arrays similarily. If you want to do error checking then do it with some other pointer, such as

if( strncpy(sYear,sFileName+3,2) != sYear)
{
   // error
}