-1

Here is the code:

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

#define numOfStrings 10
#define sizeOfString 30

void crashControl();
int main()
{
    char **strArray = (char **)malloc(numOfStrings*sizeof(char *));
    crashControl(strArray);

    for (int i = 0; i < numOfStrings; i++)
    {
        strArray[i] = (char *)malloc(sizeOfString*sizeof(char));
        crashControl(strArray[i]);
    }

    return 0;
}

void crashControl(char *A)
{
    if (!A)
    {
        printf("Not enough space.\n");
        exit(1);
    }
}

It seems work correctly. When I increase numOfStrings too much crashControl(strArray) works correctly. Likewise when I increase sizeofString too much crashControl(strArray[i]) works correctly as well. But I wonder, am I doing right or wrong? Is there a risk or a bug for this code? The parameter of crashControl() function has one dimensional array, am I free to use this function for any N-dimensional array?

Atreidex
  • 338
  • 2
  • 15
  • 1
    **Multi-dimensional arrays don't exist in C** (but you might have arrays of arrays, or arrays of pointers, or arrays of aggregate types, not only arrays of scalars). Consider some better approach, like [here](https://stackoverflow.com/a/47235897/841108). And it is much wiser to check failure of `malloc` in the same function that calls it, so your `crashControl` is very bad taste. – Basile Starynkevitch Dec 09 '17 at 10:16
  • 3
    Why not pass the `size` as an argument and allocate `size` within your function. There is a common `xmalloc` routine that provides an example (just search). – David C. Rankin Dec 09 '17 at 10:18
  • 1
    Btw look at 99% of C code and you'll see how people handle a NULL from *malloc()* (at least, look at the ones who check the returned value!) – Déjà vu Dec 09 '17 at 10:24
  • 1
    Your first call to `crashControl` is a constraint violation. If you don't see an error message then please look into invoking your compiler properly – M.M Dec 09 '17 at 10:41
  • Thank you all. A 2-dimensional array is just a pointer of pointers. Thus, I thought it would be done because the function paramater is just an pointer. – Atreidex Dec 09 '17 at 10:58
  • @BasileStarynkevitch: `crashControl` is a routine that checks the failure of `malloc` in the same function that calls it. It is a routine to provide the convenience of checking malloc, reporting an error, and terminating the program in one package. It is not like some unrelated routine checking whether a distant earlier `malloc` failed. – Eric Postpischil Dec 09 '17 at 12:25

1 Answers1

0

Yes, you may use it for multidimensional arrays. However, it could be good to rewrite it in that case. Instead of taking a char * as argument you should take a void * to make it general.

What your function is testing is basically nothing more than if the pointer is a null pointer or not. Nothing more, nothing less. I'd say it is pretty bad designed since that function gives the impression that it does more. I cannot see any reason to do a null check in a separate function. It is better to just do the check immediately after malloc, like this:

char **strArray = (char **)malloc(numOfStrings*sizeof(char *));
if(!strArray) {
    printf("Error allocating memory\n");
    exit(1);
}

If you really want to do this check in a function I would suggest naming it isNull and write it like this:

int isNull(void * ptr) 
{
    if(ptr)
        return 0;
    else
        return 1;
}

but since this function always would be called within an if statement it is pretty pointless.

klutt
  • 30,332
  • 17
  • 55
  • 95
  • The routine is clearly intended to print an error message and exit. There is no reason to remove those from the routine, and doing so subverts the purpose and requires code to be unnecessarily repeated elsewhere. – Eric Postpischil Dec 09 '17 at 12:22