0

This is all developed with Visual Stodio 2022 on a Windows 10 platform.

I am having some problems getting the library function realloc() to work correctly. I have the function adjustRGBA(), which amongst other things, removes the alpha channels from a 32-bit raw RGBA image buffer, and either places it in a separate buffer, or appends the alpha data after the RGB data. This function has a number of options, the most important being 3, which takes the alpha channel at offset 3 in a pixel after the RGB channels, and 4, which takes the alpha channel at offset 0 in a pixel before the RGB channels, then copies the alpha data to a separate buffer, or appended to the RGB data in the buffer. I don't know of any images with the alpha channel before the RGB channels in a pixel, but there is no harm in allowing for this option.

This function works correctly and as far as I can see, has been debugged.

The problem I have having is using realloc() in the function tsujdaRGBA(), which performs the reverse of adjustRGBA(), and re-inserts the alpha data either from a separate buffer, or from an offset in the same buffer.

The header contains the following code with the function prototypes:

#pragma once

#include <stdio.h>
#include <stdint.h>
#include <stdbool.h>
#include <stdlib.h>

#define SHADE 0

typedef uint8_t BYTE;
typedef const uint8_t CBYTE;
typedef const int32_t CINT;

int adjustRGBA(int, BYTE, BYTE, BYTE*, int*, BYTE*);
int tsujdaRGBA(int, BYTE, bool, BYTE*, int, BYTE*);

In the main function I have the following test code:

#include "main.h"

int main(void) {

    // 32 bytes of test RGBA data.
    BYTE testBytes[] = { 0,   1,   2, 255,   // 0
                         4,   5,   6, 255,   // 1
                         8,   9,  10, 255,   // 2
                        12,  13,  14, 255,   // 3
                        16,  17,  18, 255,   // 4
                        20,  21,  22, 255,   // 5
                        24,  25,  26, 255,   // 6
                        28,  29,  30, 255 }; // 7

    // 8 bytes of that when selected will hold the alpha data.
    BYTE testAlpha[] = { 0, 0, 0, 0, 0, 0, 0, 0 };

    int bufLen = 24;        // Set it to 24 for a separate alpha buffer or 32 to append. 
    after the RGB data.
    int alphLen = 0;        // Initialization.
    BYTE shade = SHADE;     // Initialization.
    BYTE option = 3;        // Can be 0, 1, 2, 3 or 4, but only 3 and 4 are valid for tsujdaRGBA().

    int alphOffset = 32 / 4 * 3;
    int adjustRGBA_ret = 0;
    if (bufLen == 32)
        adjustRGBA_ret = adjustRGBA(32, option, shade, testBytes, &alphLen, testBytes + alphOffset);
    else
        adjustRGBA_ret = adjustRGBA(32, option, shade, testBytes, &alphLen, testAlpha);

This works correctly. When bufLen is 32, the output is written to the input buffer, overwriting it and appending the alpha data at the end. When bufLen is 24, the alpha data is written to a separate buffer.

The problem is when I call tsujdaRGBA() with bufLen set to 24 as follows:

    int tsujdaRGBA_ret = 0;
    if (bufLen == 32)
        tsujdaRGBA_ret = tsujdaRGBA(bufLen, option, false, testBytes, alphLen, testBytes + alphOffset);
    else
        tsujdaRGBA_ret = tsujdaRGBA(bufLen, option, true, testBytes, alphLen, testAlpha);

    return 0;
}

The input arguments for the following function are:

  • bufLen: length of input RGB buffer,
  • opt: 3 or 4,
  • difBuf: true if the alpha data is in a separate buffer, otherwise false when it is appended to the input RGB data,
  • buf: The inut RGB buffer,
  • alLen: the length of the alpha data,
  • alBuf: The input alpha buffer, which is either appended to buf when dufBuf is false, or a separate buffer when difBuf is true.

The buffer buf is also the output buffer that overwrites the input data.

The function is as follows, where a negative return value indicates an error:

int tsujdaRGBA(int bufLen, BYTE opt, bool difBuf, BYTE* buf, int alLen, BYTE* alBuf) {
    if (buf == NULL) return -1;
    if ((bufLen % 3 != 0 && difBuf) || (bufLen % 4 != 0 && !difBuf)) return -2;
    if (opt != 3 && opt != 4) return -3;
    if (alBuf == NULL) return -4;
    CBYTE alOff = opt == 3 ? 3 : 0;
    CBYTE loOff = alOff == 3 ? 0 : 1;
    CBYTE miOff = alOff == 3 ? 1 : 2;
    CBYTE hiOff = alOff == 3 ? 2 : 3;

    // Calculate the length of the temporary buffer and allocate its memory.
    CINT tempLen = difBuf ? bufLen + alLen : bufLen;
    BYTE* tempBuf = NULL;
    if ((tempBuf = (BYTE*)calloc(tempLen, 1)) == NULL) return -5;

    int j = 0;  // Counts the RGB bytes.
    int k = 0;  // Counts the alpha bytes.
    // Main loop that copies the RGB buffer to a temporary buffer, and inserts the alpha data into it.
    for (int i = 0; i < bufLen; i += 4) {
        tempBuf[i + alOff] = alBuf[k++];
        tempBuf[i + loOff] = buf[j++];
        tempBuf[i + miOff] = buf[j++];
        tempBuf[i + hiOff] = buf[j++];
    }

    // Optionally allocate more memory for buf.
    if (difBuf) {
        BYTE* temp = (BYTE*)realloc(buf, tempLen); // <== Error is here.
        if (temp != NULL) buf = temp;
        else return -6;
    }

    // Copy tempBuf to buf, free tempBuf and return.
    for (int i = 0; i < tempLen; i++) {
        buf[i] = tempBuf[i];
    }
    free(tempBuf);

    return j + k; // Returns the sum of the RGB and alpha bytes.
}

There are a number of print statements and comments in this function and in main() which are omitted here.

There is no problem when the input buffer has 32 bytes, the first 24 of which contain the RGB data, followed by 8 bytes with the alpha data. The alpha data is correctly inserted in the temporary buffer, which is then copied to the original buffer, which is used for the output. The problem is when the input buffer only contains the RGB data, and has to be increased from 24 to 32 bytes before the alpha data is merged with it.

A real applications will of course have a large number of pixels, but it much easier to debug a small toy example first. Could some kindly find how the problem with realloc() can be fixed.

7
  • Are you sure that in (BYTE*)realloc(buf, tempLen) the buf pointer was previously allocated by one of alloc/calloc/realloc? Commented May 27 at 7:19
  • 7
    you pass testBytes as buf, which is a (pointer to a) statically allocated array. You can't call realloc on a statically allocated array. Commented May 27 at 7:25
  • 2
    For future questions please don't forget to try and create a single and coherent minimal reproducible example to show us. Commented May 27 at 7:41
  • 1
    typedef uint8_t BYTE; Please don't invent icky type systems like that, use the standard names. With these typedefs you are only making the code less readable and for no good reason. Commented May 27 at 7:46
  • 2
    The effect of buf = temp will not be visible outside the function it appears in. This is a frequent beginners' error. There are many duplicate Q&A about this specific mistake, see for example this or this or this or just search. Commented May 27 at 10:11

2 Answers 2

7

In C, arrays need to either have a static or dynamic size, they cannot have both at once. Therefore you may only realloc arrays that you allocated with malloc/calloc/realloc.

You attempt to call realloc on the static sized array testBytes which will not work - it is undefined behavior.

There are two ways you can fix this:

  • Either make testBytes large enough to cover all use cases (faster code but potentially more memory consuming),
  • Or allocate testBytes with malloc and populate it through memcpy from a static sized array.
Sign up to request clarification or add additional context in comments.

6 Comments

Many thanks, that was very helpful. In the application in my code, indeed I use malloc() or calloc() to allocate dynamic memory, but to make testing easier for my toy application, I just defined a fixed array. I will change that and see if the error goes away.
That fixed the bug with realloc(), but I'm getting some strange results. In the main loop in tsujdaRGBA() where I merge the alpha data with the RGB data, there was an error where I iteracted over bufLen, but this should have been tempLen, which I fixed. If I print each element of buf in the last loop at the end, it works correctly and inserts 255 for the alpha channel after the RGB channels for all 32 bytes. If the alpha data is read from the appended part of the buffer, the returned values in buf are the same and correct. However, if I merge the alpha values from a separate buffer, ...
and although buf has the correct values inside tsujdaRGBA(), they are not correct after being returned to main(). The first few lines printed for RGBA are 0, 1, 2, 4 | 5, 6, 7, 8 etc. This continues until the last 8 lines, 24, 25, 26, 255 | 28, 29, 30, 255 which are correct. In the function buf has all the correct value as 0, 1, 2, 255 | 4, 5, 6, 255 etc. to the end of the output. I want to know why buf is not being correctly returned. On a side note, if I use memcpy() to copy the buffer, it works correctly but get the warning C6387. It does not affect execution, but is an irritation.
When the alpha data is read from a separate buffer and merged with the RGB data in tsujdaRGBA(), the length of the input RGB data in buf is specified as 24 bytes, then realloc() is called to increase the buffer length to 32 bytes in order to accommodate the alpha data. However, the length of buf is in reality 32 bytes, so instead of calling realloc(), if I just copy the bytes from alpha buffer into a temporary buffer containing all the RGBA data, then copying it to the output buffer, it works correctly. For some reason, although the all the bytes are copied correctly to buf inside the...
@csharp The comments are not the place for a new question.
|
0

I followed some links about using realloc() inside a function and modified my code. The statements in the main function have been changed to:

int tsujdaRGBA_ret = 0;
if (bufLen == 32) tsujdaRGBA_ret = tsujdaRGBA(bufLen, option, false, &testBytes, alphLen, testBytes + alphOffset);
else              tsujdaRGBA_ret = tsujdaRGBA(bufLen, option, true, &testBytes, alphLen, testAlpha);

and the function tsujdaRGBA() has is now:

int tsujdaRGBA(int bufLen, BYTE opt, bool difBuf, BYTE** buf, int alLen, BYTE* alBuf) {
  printf("--- tsujdaRGBA() ---\n");
  if (buf == NULL) return -1;
  if ((bufLen % 3 != 0 && difBuf) || (bufLen % 4 != 0 && !difBuf)) return -2;
  if (opt != 3 && opt != 4) return -3;
  if (alBuf == NULL) return -4;
  CBYTE alOff = opt == 3 ? 3 : 0;
  CBYTE loOff = alOff == 3 ? 0 : 1;
  CBYTE miOff = alOff == 3 ? 1 : 2;
  CBYTE hiOff = alOff == 3 ? 2 : 3;

  // Calculate the length of the temporary buffer and allocate its memory.
  CINT tempLen = difBuf ? bufLen + alLen : bufLen;
  BYTE* tempBuf = NULL;
  if ((tempBuf = (BYTE*)calloc(tempLen, 1)) == NULL) return -5;

  // Main loop that copies the RGB buffer to a temporary buffer, and inserts the     alpha data into it..
  int j = 0;    // Counts the RGB bytes.
  int k = 0;    // Counts the alpha bytes.
  for (int i = 0; i < tempLen; i += 4) {
    tempBuf[i + alOff] = alBuf[k++];
    tempBuf[i + loOff] = (*buf)[j++];
    tempBuf[i + miOff] = (*buf)[j++];
    tempBuf[i + hiOff] = (*buf)[j++];
  }

  if (difBuf) {
    printf("--- realloc() ---\n");
    BYTE* temp = (BYTE*)realloc(*buf, tempLen);
    if (temp != NULL) *buf = temp;
    else {
      free(tempBuf);
      return -6;                                            
    }
    **buf = *tempBuf;
  }

  printf("Copy from tempBuf\n");
  for (int i = 0; i < tempLen; i++) {
    (*buf)[i] = tempBuf[i];
    printf("%4d %02X\n", i, (*buf)[i]);
  }
  free(tempBuf);

  return j + k; // Returns the sum of the RGB and alpha bytes.
}

The return value from realloc() is saved in a temporary pointer *temp, so that in case NULL is returned, *tempBuf can be freed then the function returns with an error code, otherwise **buf is set to *tempBuf, then later on is contents are copied to **buf, which is returned. If a temporary buffer is not used to hold the return from realloc() I get a warning about not handling a NULL return value.

Printing the values in the loop in the function, matches the output when printing the values after the function has returned agree, and are correct, i.e. the buffer now has RGBA, RGBA, etc.

When 24 is selected as the number of bytes in the input buffer containing only the test RGB data, realloc() is called to increase the buffer size to 32 to allow the alpha values to be included. When 32 is selected, realloc() is not called as the buffer already includes the alpha values and does not need to be changed. Both now work correctly as far as I can see.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.