uint8_t array return is causing errors using all scenarios (**, *, &, nothing)

Question

int8_t** FileToColorMap(char* colorfile, int* colorcount)
{
    FILE* fp = fopen (colorfile, "r");
    if (fp==NULL) 
    { 
        printf("no such file."); 
        return 0; 
    } 

    int r,g,b;
    fscanf(fp, "%d", colorcount);
    uint8_t* output;
    output = (uint8_t *)malloc(*colorcount * sizeof(uint8_t));
    for (int i = 0; i < *colorcount;i++) {
        if( fscanf(fp, "%d %d %d", &r, &g, &b) != EOF ) {
            // int* arr= (int *)malloc(3 * sizeof(int));
            // arr[0] = r;
            // arr[1] = g;
            // arr[2] = b;
            int arr[3] = {r,g,b};
            output[i] = *arr;
        } else
        {
            freeMap(i+1, &output);
            return NULL;
        }
        
    }
    fclose(fp);
    return *output;
}

This causes seg fault error, or error 1 or something even when I try return output, **output, &ouput.

It looks like your post is mostly code; please add some more details.

Answer 1

There are many issues:

1. Wrong type and allocation

uint8_t* output;
output = (uint8_t *)malloc(*colorcount * sizeof(uint8_t));

You need to declare pointer to pointer here and allocate space for the pointers not uint8_t

Wrong types of both arrays. arr is automatic variable and dereferencing it outside the function scope is an UB

• many other issues

int8_t** FileToColorMap(char* colorfile, int* colorcount)
{
    FILE* fp = fopen (colorfile, "r");
    if (fp==NULL) 
    { 
        printf("no such file."); 
        return 0; 
    } 

    int8_t r,g,b;
    if(fscanf(fp, "%d", colorcount) != 1)
    {
       /* fscanf failed - do something */
    }
    int8_t **output = malloc(*colorcount * sizeof(*output));
    for (int i = 0; i < *colorcount;i++) {
        if( fscanf(fp, "%hhx %hhx %hhx", &r, &g, &b) == 3) {

            uint8_t *arr = malloc(3 * sizeof(*arr));
            arr[0] = r;
            arr[1] = g;
            arr[2] = b;
            output[i] = arr;
        } else
        {
            freeMap(i+1, &output);
            return NULL;
        }
        
    }
    fclose(fp);
    return output;
}

always check the result if malloc. I do not for the clarity sake

Answer 2

Overall the code is needlessly complicated. I would propose to use a struct instead, since the inner-most dimension in this case is always 3 items wide.

Assuming color_count is allocated by the caller and returns the number of items read, then:

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

typedef struct
{
  uint8_t r;
  uint8_t g;
  uint8_t b;
} rgb_t;

rgb_t* FileToColorMap (const char* color_file, int* color_count)
{
  FILE* fp = fopen (color_file, "r");
  if (fp==NULL) 
  { 
    *color_count = 0;
    return NULL;
  } 
  
  int count;
  if(fscanf(fp, "%d", &count) != 1)
  {
    fclose(fp);
    *color_count = 0;
    return NULL;
  }
  
  rgb_t* result = malloc ( sizeof(rgb_t[count]) );
  if(result == NULL)
  {
    fclose(fp);
    *color_count = 0;
    return NULL;
  }
  
  for (int i = 0; i<count; i++) 
  {
    int r,g,b;
    if( fscanf(fp, "%d %d %d", &r, &g, &b) != 3 )
    {
      free(result);
      fclose(fp);
      *color_count = 0;
      return NULL;
    }
    
    result[i].r = r;
    result[i].b = b;
    result[i].g = g;
  }

  fclose(fp);
  *color_count = count;
  return result;
}

Note:

• const correctness added to the file name parameter.
• Proper clean-up both of dynamic memory and the file handle upon errors.
• Use a temporary internal variable count until you know that everything went well. Write to the caller's variable in the end of the function.
• Check that the result of fscanf corresponds to the expected number of items, not just that it isn't EOF.

---

Advanced:

To clean up the code further, one needs to look at the error handling and resource clean-up. There's lots of repetition in the error handling in the above code, which isn't ideal - clean-up should be centralized at one location or it is easy to get resource leaks, particularly during maintenance later on.

There's three ways to do that: a clean-up macro, an "on error goto" pattern or a wrapper function. I prefer the latter since it is the least messy. It's done by breaking out the ownership of resources to an outer function, the one that the caller knows about. Then you put the actual algorithm inside an internal static function.

Here's an example of that, with centralized clean-up/error handling:

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

typedef struct
{
  uint8_t r;
  uint8_t g;
  uint8_t b;
} rgb_t;

static bool FileReadColorMap (FILE* fp, rgb_t** result, int* color_count)
{
  int count;
  fscanf(fp, "%d", &count);
  
  *result = malloc ( sizeof(rgb_t[count]) );
  if(*result == NULL)
  {
    return false;
  }
  
  for (int i = 0; i < count; i++) 
  {
    int r,g,b;
    if( fscanf(fp, "%d %d %d", &r, &g, &b) != 3 )
    {
      return false;
    }
    
    (*result)[i].r = r;
    (*result)[i].b = b;
    (*result)[i].g = g;
  }

  *color_count = count;
  return true;
}

rgb_t* FileToColorMap (const char* color_file, int* color_count)
{
  rgb_t* result = NULL;
  FILE*  fp     = NULL;
  
  fp = fopen (color_file, "r");
  if(fp == NULL)
  {
    *color_count = 0;
    return NULL;
  }
  
  if(!FileReadColorMap(fp, &result, color_count))
  {
    *color_count = 0;
    free(result);  // it is safe to do free(NULL)
    result = NULL; // return NULL
  }
  
  fclose(fp); // always called
  return result;
}