Issue in Dynamic Memory Allocation

Question

I am writing a function to find peak in an array in a given range. I don't know the exact number of peaks in the given array. So I am using Dynamic memory allocation to store the peaks. The problem I am facing is that ,

 int *findPeak(float *pArray, int length, int window, int *pCount) {
     int i, j ,count = 0, toPositive = 0, toNegative = 0;
     float peak;
     int *pPeakBuffer;
     bool firstZeroCross = false, toPositivePeak = false;
     int *peakBuffer = (int*)malloc(1*sizeof(int));
    
     for (i = 0; i < length; i += window) {
         if (count == 0) {
             peak = 0.0;
             for (j = i; j < window; j++) {
                 if (peak < pArray[j]) {
                     peak = pArray[j];
                     peakBuffer[count] = j;
                 }          
             }
             printf("Peak = %d\n\r", peakBuffer[count]);
             count++;
         }
         else {
             peak = 0.0;
             peakBuffer = (int*)realloc(peakBuffer, 1*sizeof(int)); 
             for (j = i; j < i+window; j++) {
                 if (peak < pArray[j]) {
                     peak = pArray[j];
                     peakBuffer[count] = j;             
                 }          
             }
             printf("Peak = %d\n\r", peakBuffer[count]);
             count++;
         }
     }
     *pCount = count;
     printf("count = %d\n\r", count);
     for (i = 0; i < count; i++)
         printf("%d ,", peakBuffer[i]);
     printf("\n\r");
     return peakBuffer;
 }

when a peak value is detected and it store it in the first memory. When the 2nd peak is detected it store in the 2nd memory, then the value in the first memory(previous memory) is changed to zero or other numbers and so on. Only the first and last peak is obtained. rest of the memory store some unknown values(not the peak value). I don't know why it is happening.

`realloc(peakBuffer, 1*sizeof(int))` You are giving the _same_ size every time. You need to increase the size by at least 1 int each time to accomodate more entries. — kaylum, Sep 01 '22 at 05:28
The size passed to [`realloc`](https://en.cppreference.com/w/c/memory/realloc) is the *new* size, not how much to add or take away. — Some programmer dude, Sep 01 '22 at 05:28
Also note that `realloc` can fail and return a `NULL` pointer. It will leave the original memory though, so if you reassign back to the pointer you pass you will loose the original memory. Always use temporary variables, and always check for failure. And [don't cast the result of `malloc` (or `realloc`)](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858). — Some programmer dude, Sep 01 '22 at 05:30
Also see [Proper usage of realloc()](https://stackoverflow.com/questions/21006707/proper-usage-of-realloc) — yano, Sep 01 '22 at 05:31
If all you want is a single `int`, there's no point in dynamically allocating any memory, just do `int peakBuffer` and overwrite it each time. By passing the size of one `int` into `malloc` and `realloc`, you never allocate space for more than one `int`. — yano, Sep 01 '22 at 05:36
All you do is keep `peakBuffer` the same size, containing only a single element. But you increase `count` (possible more than you assign to the elements). So all while `peakBuffer[0]` is the only single element in the array, you still access `peakBuffer[1]`, `peakBuffer[2]`, etc. — Some programmer dude, Sep 01 '22 at 05:37

Fe2O3 · Answer 1 · 2022-09-01T06:55:00.270

Apart from the memory allocation issues, and the boundary issues, and the "\n\r" misunderstanding, you've made an attempt worthy of what may be a helpful response.

It would take all day to dissect the code in your OP and discuss what's not quite right. Sorry.

The code below is adapted from yours, with issues sorted out and imposing what seems to be the objective of your pursuit. It seems that your code would have attempted to store the indices of successive increasing values found in any window... Really? That would be a lot of indices!!

This function searches for the single maximum value within any window and 'logs' only the indices (within the entire array, not just a 'window') of those maximums, one maximum for each window.

Perhaps this will provide you the functional foundation to achieve your objective. (I've compiled this, but have not tested it with any data. Offered "as is"...

int *findPeak( float *pArray, int length, int window, int *pCount ) {
    int i, count = 0;
    int *pPeaks = NULL; // ALWAYS initiase pointers

    for( i = 0; i < length; i += window ) {
        int peak = i; // assume peak is the first one in this window

        // scan window finding highest peak
        // Notice that 'j' starts with index of beginning of window
        // Notice limit to not run beyond length
        for( int j = i; j < i + window && j < length; j++ )
            if( pArray[ j ] > pArray[ peak ] ) // compare two floats values
                peak = j; // remember higher peak

        // get room to store this array index
        int *tmp = (int*)realloc( pPeaks, (count + 1) * sizeof *tmp );
        if( tmp == NULL ) {
            /* deal with failure */
            exit( EXIT_FAILURE );
        }
        pPeaks = tmp; // did not fail

        pPeaks[ count++ ] = peak; // index relative to entire array
    }

    // Pointless because this version will find one peak in each window
    // Could be calculated by dividing length by window
    *pCount = count;
    printf( "count = %d\n", *pCount );

    for( i = 0; i < count; i++ )
        printf( "%d, ", pPeaks[ i ] );
    putchar( '\n' );

    return pPeaks;
}

Issue in Dynamic Memory Allocation

1 Answers1