Creating a buffer inside a function and assigniing values to its elements in C-CodePudding

I have a new question that is related to my posting at the link Allocating dynamic memory in C in a functio for creating a uniform BMP image with specified colors for some tests. As before I'm using Visual Studio 2022 on a Windows 10 platform.

In the main function I have the following code:

int numCheckBytes;
BYTE* bufPix = NULL;
CINT numPixBytes = createImgPix(width, height, red, green, blue, &numCheckBytes, &bufPix);

where the width and height in pixels are specified, red, green and blue are bytes with values between 0 and 255, numCheckBytes is a returned value, and bufPix is a buffer declared as NULL before the function is called.

Inside the function createImgPix() I have the following code:

CINT createImgPix(int width, int height, CBYTE rd, CBYTE gn, CBYTE bl, int* countChk, BYTE** buf) {
  if (*buf != NULL) return -1; // Buffer already has something in it.
  CBYTE color[] = { bl, gn, rd }; // A BMP image has the colors in reverse order, i.e. as blue, green and red.
  int pixVer = 0;
  int pixByte = 0;
  int byteCount = 0;
  CINT padding = (4 - ((width * 3) % 4)) % 4; // Number of padding bytes at the end of each row.
  CINT pixByteRow = width * 3; // Number of pixels bytes in a row.
  CINT rowBufLen = pixByteRow   padding; // Total number of bytes in a row including any padding.
  CINT bufLen = rowBufLen * height; // Total number of bytes including any padding.
  int offset = 0;

  // Allocate a buffer to hold all the pixel data.
  *buf = (BYTE*)malloc(bufLen);
  if (*buf == NULL) return -2; // Failed to allocate memory.

  for (pixVer = 0; pixVer < height; pixVer  ) { // Loop over rows.

      for (pixByte = 0; pixByte < pixByteRow; pixByte  ) { // Loop over pixels in a row.
          *(buf   pixByte) = color[pixByte % 3]; // <== (1)
          byteCount  ;
      }

      for (int i = 0; i < padding; i  ) { // Add any padding bytes.
          pixByte = 3 * width   i;
          *(buf   pixByte) = 0; // <== (2)
          byteCount;
      }

      offset  = rowBufLen; // Increment offset by number of bytes in a row.
  }

  *countChk = byteCount - 3 * width * height - height * padding;    // Check total bytes counted against image size.
  return byteCount;
}

The idea is to allocate the buffer inside the function, populate it with values, then return the buffer with its populated values to the main function. When compiling the application I get the warning message for (1) as "warning C4047: '=': 'BYTE *' differs in levels of indirection from 'CBYTE'". I get no warning for (2), but presumably there is a similar problem. Note that in the header I have:

typedef unsigned char BYTE;
typedef const unsigned char CBYTE;

If for (1) I have instead the statement:

*buf[pixByte] = color[pixByte % 3];

the program compiles correctly but an exception is thrown at that line when the code is executed.

In an earlier version of this function I created a temporary buffer inside it with the line:

buf = malloc(rowBufLen);

and line (1) was instead:

buf[pixByte] = color[pixByte % 3];

which worked correctly. In that case I wrote to a buffer for each line scanned, copied the buffer to a file, then reused the buffer for the next lines, etc. discarding it when returning from the function, with the image written to a file. Now I want the pixel part of the image kept in a buffer, which can be accessed from main.

Getting some help with this would be most appreciated. Incidentally, I always document my code with comments, but they don't come out well here.

CodePudding user response：

At this line:

*(buf   pixByte) = color[pixByte % 3];

You have buf which is a BYTE** and (presumably) contains the address of a variable defined in the calling function. When you add pixByte to this, you're actually offsetting the address of that variable instead of indexing the memory block that was returned.

This is close:

*buf[pixByte] = color[pixByte % 3];

But the array index operator has higher precedence than the dereference operator. What you actually want is:

(*buf)[pixByte] = color[pixByte % 3];

The other line has the same problem. The reason you don't get an error is because 0 is a valid null pointer constant.

CodePudding user response：

OK, many thanks, I got this to work. It's one of the nuances of C that I missed. I forgot to account for the offset, so the relevant part of the code is now:

//...
int offset = 0;

// Allocate a buffer to hold all the pixel data.
*buf = (BYTE*)malloc(bufLen);
if (*buf == NULL) return -2;

for (pixVer = 0; pixVer < height; pixVer  ) {

    for (pixByte = 0; pixByte < pixByteRow; pixByte  ) {
        (*buf)[pixByte   offset] = color[pixByte % 3]; // <== (1)
        byteCount  ;
    }

    for (int i = 0; i < padding; i  ) {
        pixByte = 3 * width   i;
        (*buf)[pixByte   offset] = 0; // <== (2)
        byteCount;
    }

    offset  = rowBufLen;
}
//...

with (1) and (2) changed with the offset included. Now indeed I do get a uniformly colored image when combined with the header and written to a file.

However, the IDE shows (1) underlined in green with the following warning when mousing-over it "C6386: Buffer overrun while writing to '*buf'". The buffer doesn't overrun, so I don't know the reason for this, any ideas? For (2) I get no such message, perhaps because, as you said, this is the same as setting the elements to NULL. The code still needs to be cleaned up and tested a bit more.

I don't understand the comment "No, you don't" from wildplasser about if (*buf == NULL), what does this mean? If for some reason the memory is failed to be allocated, I want to return immediately with a negative number to indicate failure - many thanks.