1

I am trying to write a function that breaks a char** math expression into its constituents. For example, "(10 - 2) / 2" would become { (, 10, -, 2, ), /, 2 }.

Because of the problem description I have been given, the return type must be char** (a pointer to a character pointer, but essentially an array of strings). I am a beginner in C/++ so I am fairly unfamiliar with pointers and memory management at the moment.

char* intTocharStar(int x) {
    int length = snprintf( NULL, 0, "%d", x );
    char* str = (char*)malloc( length + 1 );
    snprintf( str, length + 1, "%d", x );

    return str;
}

char* convertChar(char c) {
    char* pointer = (char*)malloc(2*sizeof(char));
    pointer[0] = c;
    pointer[1] = '\0';

    return pointer;
}

char **tokenize(const char* input) {

    char **arr = (char**)malloc(1024 * sizeof(char*));
    char op;
    int num;
    int index = 0;

    stringstream parser(input);
    while (parser) {

        if (isalnum(parser.peek())) {

            parser >> num;
            cout << "Num " << num << '\n';

            //args[index] = intTocharStar(num);
            strcpy(arr[index], intTocharStar(num));
        
        } else {
            parser >> op;
            cout << "Op " << op << '\n';

            //args[index] = convertChar(op);
            strcpy(arr[index], convertChar(num));
        }
        index++;
    }

    char** res = (char **)malloc(100 * sizeof(char*));

    // remove final element, which is a duplicate
    for (int i = 0; i < index - 1; i++)
        res[i] = arr[i];

    free(arr);

    return res;

}

When I run this code, the program stops abruptly the first time the for loop runs. I decided to try debugging the program and got a segmentation fault on this line:

arr[index] = intTocharStar(num);

This is where the int parsed from the input is meant to be added to the output char** as a char*. After reading around here I tried changing this to:

strcpy(arr[index], intTocharStar(num));

But I still get the same segmentation fault. I also wrote a short program where a char* is simply strcpy'd into the first index of a char** in the main function and got this same segmentation fault, so I believe the problem is either that line where arr is accessed or this line where it is declared:

char **arr = (char**)malloc(1024 * sizeof(char*));

However from what I've read this seems to be the standard way of declaring a pointer and allocating memory to it. Is there anything I am overseeing or is there a way to further debug this?

user17732522
  • 53,019
  • 2
  • 56
  • 105
Dunkler
  • 11
  • 2
  • 4
    Your code is clearly not C, but C++, since you are using `cout <<`, but in C++ you should not be using `malloc` like this at all. The `malloc` "replacement" in C++ is `new`. But instead of either you really should just be using `std::string` and `std::vector`. Who/what taught you to use `malloc` here? – user17732522 Jun 19 '22 at 15:40
  • 2
    There is no variable named `args`. When using the assignment `arr[index] = intTocharStar(num);`, it works as expected. – Olaf Dietsche Jun 19 '22 at 15:49
  • `sizeof(char)` is per definition 1. – Goswin von Brederlow Jun 19 '22 at 15:50
  • What you are overseeing is decades of C++ development. You are learning C disguised as C++ by sprinkling in a few cout here and there. Learn about `std::string` and `std::vector` and `std::variant` and enums. – Goswin von Brederlow Jun 19 '22 at 15:53
  • @user17732522 Yes this is C++ code, I didn't mean to tag it C, thank you for removing the tag. I believe when I first had the need to use pointers I mainly came across C resources and and then came across posts like https://stackoverflow.com/questions/3477741/why-does-c-require-a-cast-for-malloc-but-c-doesnt?noredirect=1&lq=1 that lead me to believe using malloc in C++ was standard. I didn't think to use 'new'. The reason I don't use string or vector is because I mentioned, the function must return char** so I would have to convert it at some point. – Dunkler Jun 19 '22 at 16:00
  • "the function must return char**" If this requirement is not spelled out in your assignment, then it's false. – n. m. could be an AI Jun 19 '22 at 16:04
  • @Dunkler `malloc` is not _technically_ wrong here, but shouldn't be used in C++ in preference for `new` (or proper RAII-style classes). It will be wrong for non-trivial types. There are specialized cases where `malloc` might be used directly, but they shouldn't concern you as beginner. When learning C++ you should be careful that although a large part of C is also valid C++, the actual style, conventions, idioms and design are very different. If the problem is asking you to return a `char**` which has to be freed by the caller, that is not how C++ functions are designed. That is C (at best). – user17732522 Jun 19 '22 at 16:05
  • Don't learn C++ From the Internet. Not even from Stack Overflow. Until you know the fundamentals, you'll have no good way to tell good information from bad or appropriate from inappropriate. [Get the basics from a good book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). – user4581301 Jun 19 '22 at 16:06
  • 1
    If it has to return `char**`, then you rally should still work with `std::string` and only convert at the very end. Note that there is `const char* std::string::c_str()` which allows easy conversion. Consider putting the converted words into a `std::vector` and then use `std::vector::data` for another easy conversion. Don't let the required return type force you to write bad code. – Aziuth Jun 19 '22 at 16:06
  • @Aziuth That doesn't work, because the owning `std::vector`s will be destroyed when the function returns. Asking to return a `char**` pointing to an array of C-style strings fundamentally requires direct allocation with `new[]`/`malloc` and deallocation with `delete[]`/`free`. This imposes the problem that the function _must_ use the corresponding allocation method to the deallocation method that the caller is using. If the caller uses `free`, the function must use `malloc`. If the caller uses `delete[]`, the function must use `new[]`. Which of these it is, is also lacking from OP. – user17732522 Jun 19 '22 at 16:10

1 Answers1

0

There are few big mistakes you make here, but I will explain only one which will probably fix all you memory issues.

char **arr = (char**)malloc(1024 * sizeof(char*));

You wanted to allocate matrix of char while what you really did was to allocate an array of pointers to pointers to char. This mean you will allocate 1024*8(if this is 64 bit compiler) pointers to pointers to char. Instead of allocating enough of place to 1024 strings, you allocated space to pointers. Better way to do it is to allocate an array of strings:

char** arr= new char*[1024];

And for each string you should allocate memory. For Example:

arr[0] = new char[20];

Of course, in the end you should deallocate each string you allocated and the arr variable.

delete arr[0]; //delete one string
delete[] arr; // delete arr char**

In general you better convert it to array of strings and use strings instead of char** (which will fix your need of managing the memory).

Ziv Kvatinsky
  • 15
  • 2
  • `std::vector` in this case to completely eliminate all of the memory management woes. – user4581301 Jun 19 '22 at 19:36
  • Yes this was the problem, thanks, though I can't upvote yet because I don't have enough rep. 273K suggested something similar before deleting their answer, but sticking with malloc to initialize the array elements: `arr[index] = (char*)malloc(12);` Also, would you mind outlining the other problems you noticed in the code? – Dunkler Jun 19 '22 at 19:49
  • If you will do it you will have a lot of memory leaks because your functions also allocate memory. – Ziv Kvatinsky Jun 20 '22 at 20:07
  • You should use these functions and use then with arr[index]= func() – Ziv Kvatinsky Jun 20 '22 at 20:08