4

I have code this add_str function :

void    add_str(char *str1, char *str2, char **res)
{
  int   i;
  int   j = 0;

  res[0] = malloc((strlen(str1) + strlen(str2) + 1) * sizeof(char));
  if (res[0] == NULL)
    return;
  for (i = 0; str1[i]; i++)
    res[0][j++] = str1[i];
  for (i = 0; str2[i]; i++)
    res[0][j++] = str2[i];
  res[0][j] = '\0';
}

It receives 2 strings, str1 and str2 and a pointer an string **res which is not malloc. My function add str1 and str2 on **res.

My question is : Is there a way to do not write res[0] each time I have to do something with it ?

Improve this question
3
  • 1
    char *temp = res[0]; ? Commented Jan 9, 2014 at 9:41
  • 3
    Also, you do know about strcpy and strcat? Commented Jan 9, 2014 at 9:43
  • use strcpy and strcat Commented Jan 9, 2014 at 9:50

5 Answers 5

Reset to default
6

res is a pointer to a pointer, so you need to dereference it before you use it. You're right that res[0] isn't the right way to do that though in this context. Use (*res) instead.

Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

Or you might as well just pass 'char* res' instead of 'char** res'... And in addition, you can use strcpy and strcat instead of the two 'for' loops.
Well the parentheses will be needed if the OP want to continue to use the current code, as *res[j] is something different from (*res)[j].
@barakmanos Then the OP can't allocate the memory in the function.
@JoachimPileborg : You're correct about the parentheses. I'll fix my answer. Thanks!
@JoachimPileborg: I stand corrected. So it's either 'res[0]' or '*res', but I don't see any obvious reason to prefer one over the other.
|
3

What you really want is to dereference:

*res
Improve this answer

Comments

2

This is not a direct answer to your question, but more of a general coding advice:

In order to properly manage all dynamic memory operations within your program, you should strive (try as much as you can) to have both operations x=malloc(...) and free(x) executed from the same function.

In most cases, if you design your code correctly, then you can achieve this.

In some cases, when the flow is asynchronous, it is impossible to do so. For example, one function allocates the buffer, and after some event occurs in the system, another function (AKA callback) releases the buffer.

In such cases, you should still try to have both operations within the same "scope" (class, file or module).

In your coding example, function add_str allocates the buffer but does not release it, which means some other function will eventually have to do it (or you will have memory leaks). Hence, if possible, then you should try to perform the malloc outside of this function.

Improve this answer

7 Comments

That's nonsense. The whole point of the add_str function is to allocate a new string that is big enough to hold the two other strings. The calculation of required memory is neatly wrapped in the function where it is needed and hidden from the client code. Your advice would lead to cluttering the client code with such calculations. Of course it must be made clear to the user that she needs to free the memory later. Also, it could be argued that the function would ne easier to use if it returned a pointer instead of altering it via a pointer to pointer.
@M Oehm: First of all, it would NOT need to "alter a pointer to pointer", as it would receive a "normal" pointer from the user and simply fill the pointed buffer with data. Second, this is exactly how functions like strcpy, memcpy and so forth work - they DO NOT allocate a new buffer inside, but instead they receive from the user a buffer which they assume sufficiently large for safely performing the 'copy' operation.
Some people would typically look only at the current problem at hand when they try to solve it, while ignoring other problems that might emerge as a result of their solution. In the case above, the code clearly works. But on a larger scale, you would have a system with "malloc"s and "free"s scattered across the code. From my experience, you quickly "lose control" over your dynamic memory management, and you end up with several different memory leaks that you are not even aware of.
Yes, the good (?) old string operations work that way. Yes, needlessly mallocing temporaries is not good style, even if you free them. Yes, managing memory in C is hard. Yes, the design of the OP's function is quirky. Still, the standard function in question here would be strcat with possible overflow or strncat with the snag that n + 1 characters are written on overflow, the behaviour of which is inconsistent with that of snprintf. To me, it looks as if the OP saw that he needed to calculate lengths and allocate frequently and wrote a function for it. I see nothing wrong with that.
@M Ohem: That's why I said at the beginning of my answer 'This is not a direct answer to your question, but more of a general coding advice'. And if you agree with me about "the good old string operations" (such as strcpy), then I guess that you also agree with this coding advice. OP might have a particular reason for implementing it the way he did... or he might not... I gave this advice just in case the latter was the case...
|
2

Why do you need to pass a pointer pointer anyway? You could just as well change it to this:

char *add_str(char *str1, char *str2)
{
    char *res = malloc((strlen(str1) + strlen(str2) + 1));
    if (res == NULL)
        return NULL;

    char *ret = res;

    while(*str1 != 0)
        *res++ = *str1++;

    while(*str2 != 0)
        *res++ = *str2++;

    *res = '\0';
    return ret;
}

It has the same effect and you don't have to deal wit that ugly construct.

Improve this answer

2 Comments

This prototype was imposed, It's a school exercise I wanted to optimize.
Ah, I thought that this might be the case.
0

I would use another pointer variable to hold the result string, which avoids dereferencing the result variable all the time. Apart from that, of course, you should avoid using the array subscript operator where a simple pointer dereference is what you do. I have also made a few other changes to your example code to make it more concise, feel free to ignore them.

void add_str(char *str1, char *str2, char **res) {
    char* result = *res = malloc((strlen(str1) + strlen(str2) + 1) * sizeof(char));
    //Dereferencing a NULL pointer will safely crash your program on any sane system.
    //if (!result) return;
    int   j = 0;    //Since C99 you are allowed to mix code and variable declarations.
    for(int i = 0; str1[i]; i++) result[j++] = str1[i];
    for(int i = 0; str2[i]; i++) result[j++] = str2[i];
    result[j] = '\0';
}

If pointers are used to their full potential, your code can look like this:

void add_str(char *str1, char *str2, char **res) {
    char* result = *res = malloc((strlen(str1) + strlen(str2) + 1) * sizeof(char));
    while(*str1) *result++ = *str1++;
    while(*str2) *result++ = *str2++;
    *result = '\0';
}
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.