0

I want to know how to alloacte memory for a char pointer in a function using double pointer and write into the pointer.

I tried to write the following code but it crashes. What is the bug in this?

#include <stdio.h>
void myfunc(const char* src, char** dest)
{
  *dest = (char*)malloc(200);
  while(*(*dest++) = (*src++ != '\0'));
  *(*(++dest)) = '\0';
}
void main()
{
 char* src = "hello";
 char* dest = null;
 myfunc(src, &dest);
 printf("%s\n",dest);
}
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • There is no data in `dest`. First fill it with valid data and make sure it is null terminated and then do the `==` operation. – Mahesh Mar 16 '12 at 13:17
  • What are you trying to accomplish? It looks like you are trying to compare each character with no bounds checking, and you are also never setting `dest` to anything after you `malloc` it. – Joe Mar 16 '12 at 13:17
  • 1
    incrementing a `char **` will make it point to the next `char *`, not to the next `char`. – sidyll Mar 16 '12 at 13:18

3 Answers3

3

You've written a compare loop instead of a copy loop ('==' vs '='), and you are incrementing the wrong pointer when you write:

 while(*(*dest++) == *src++);

(The additional line:

 *(*(++dest)) = '\0';

is a late-breaking addition to the question. I'm not sure I want to try parsing that at all. It is not a part of the solution to the problem. See the discussion below.)

The easiest way to get that correct is probably:

 char *tgt = *dest;
 while ((*tgt++ = *src++) != '\0')
     ;

We can correct your code in phases (and I did so like this):

static void myfunc(const char* src, char** dest)
{
    *dest = (char *)malloc(200);
    char *tgt = *dest;
    while ((*(tgt++) = *(src++)) != '\0')
        ;
}

This parenthesises the expressions in the loop fully. We can now substitute *dest for tgt:

static void myfunc(const char* src, char** dest)
{
    *dest = (char *)malloc(200);
    char *tgt = *dest;
    while ((*((*dest)++) = *(src++)) != '\0')
        ;
    printf("1: %s\n", tgt);
}

And this prints 1: hello, but the main program prints an empty line because you've modified *dest so it points to the NUL '\0' at the end of the copied string. So, you'd need to do:

static void myfunc(const char* src, char** dest)
{
    *dest = (char *)malloc(200);
    char *tgt = *dest;
    while ((*((*dest)++) = *(src++)) != '\0')
        ;
    printf("1: %s\n", tgt);
    *dest = tgt;
}

And then main() will print the correct answer. But, if you're doing that dinking with tgt (an abbreviation for 'target'; I usually use dst for destination, but that is too close to your dest), you may as well avoid the complexity of incrementing *dest in the first place.

In fact, you should consider using:

#include <string.h>

...
strcpy(*dest, src);

to copy the string. Using strcpy() is probably better, as in 'faster' and 'simpler' and unequivocally correct.


Also, you should have:

#include <stdlib.h>

to declare malloc().

And the correct return type for main() is int:

int main()
{
    ...
    return(0);
}

In C99, the return is (regrettably) optional and zero (success) will be assumed if it is missing; this matches the behaviour of C++98. In earlier versions of C, the return was not optional.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
0

There are so many problems in that little snippet, I don't know where to start... To sum it up, you have made the code needlessly complex and therefore it ended up full of bugs.

Things to fix to make this code compile:

  • Unless this is code for an embedded system, or unless you are writing an operative system, main must return int.
  • NULL is an upper-case constant in C. It is found in the library stddef.h.
  • The malloc function is found in stdlib.h, which must be included.

Serious bugs:

  • Never typecast the result of malloc. More info in the C FAQ and on this SO post.
  • Always free the memory allocated by malloc.
  • You assign the boolean result (true/false) of *src++ != '\0' to a character.

Widely-recognized bad & dangerous practice that leads to bugs:

  • Always declare pointers to string literals as const.
  • Never use assignment inside conditions. (MISRA-C:2004 13.1).
  • Never use ++ operators inside complex expressions (MISRA-C:2004 12.13).
  • Never put a semicolon at the end of a row containing a loop statement. (MISRA-C:2004 14.9)
  • Never use any statement without braces {} (MISRA-C:2004 14.8).

Poor style:

  • main() should always return.
  • Avoid "magic numbers", particularly when passing parameters to malloc.
  • Always check the result of malloc().

Useful hints:

  • calloc sets all of the allocated memory to zero, unlike malloc. If you use calloc you don't have to set them to zero manually.

Fixed code:

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

#define DYNAMIC_BUF_SIZE 200

void make_string (const char* src, char** dest)
{
  *dest = calloc(DYNAMIC_BUF_SIZE, sizeof(char));

  if(*dest == NULL)
  {
    /* error handling here */
  }

  char* dst = *dest;
  *dst = *src;

  while(*src != '\0')
  {
    dst++;
    src++;
    *dst = *src;
  }
}

void delete_string (char* str)
{
  free(str);
}

int main()
{
  const char* src = "hello";
  char* dest = NULL;

  make_string (src, &dest);
  printf("%s\n",dest);
  delete_string(dest);

  return 0;
}

EDIT: New version without strcpy(), as requested by OP.

Community
  • 1
  • 1
Lundin
  • 195,001
  • 40
  • 254
  • 396
0
//It seems that you don't understand the nature of char* and char**.
char *str = "hello! I am from China and i want to make friends with foreigners";
char **ptr = {"hello!","i want to learn spoken English","sinalym@163.com"};
//Allocate memory for a char** variable. Two steps as follows:
int length[3] = {6,31,16};
char **ptr2 = new char*[3];
for(int i = 0;i < length[i];i++)
    *(ptr2 + i) = new char [length[i]];
//delete according to a reverse order.