1

I have the following code:

struct el{      
    char *line;
    struct el *next;
};
struct el *abc, def;
char *p1;
char buffer[100];
abc = &def;
gets(buffer);
p1 = malloc(strlen(buffer) + 1 );
strcpy( p1, buffer);

How can I make line point to the same string pointed by p1 without allocating the same string twice? Is it enough to write:

 abc->line = p1;

Or do I also need to free the area pointed by p1?

Andrea
  • 39
  • 5
  • 2
    1) don't use `gets` 2) `sizeof(strlen(buffer))` is completely wrong – Jean-François Fabre Aug 25 '20 at 13:22
  • `abc->line = p1` doesn't create a copy. Freeing p1 frees the other pointer too (same addresses) – Jean-François Fabre Aug 25 '20 at 13:23
  • Is there a faster function than gets ? – Andrea Aug 25 '20 at 13:30
  • 1
    Use [`fgets`](http://www.cplusplus.com/reference/cstdio/fgets/) – anastaciu Aug 25 '20 at 13:32
  • 2
    @Andrea it is not a matter of speed. It is just it is unsafe, as it doesn't perform any check on the size of the input (in your case, since `buffer` has size 100, by sending in input more than 100 chars will make gets write out of bounds, and that will probably crash your program). – Roberto Caboni Aug 25 '20 at 13:33
  • Ok thank you, i will change it immediately – Andrea Aug 25 '20 at 13:39
  • Expanding the second advice from @Jean, with `sizeof(strlen(buffer))` you are calculating the size (`sizeof`) of the value returned by strlen, that will be `sizeof (size_t)` whatever the length of the string contained in buffer is. So you will always get 8 (or 4, depending to your platform). Just use `malloc (strlen (buffer) + 1)` – Roberto Caboni Aug 25 '20 at 13:39
  • 1
    `abc->line = p1;` does exactly what you think it does. It takes the value in p1 (which is an address) and saves that same value into abc->line. Now abc->line and p1 store the same address. – user253751 Aug 25 '20 at 14:18

4 Answers4

2

I sense something smelly... Namely, that all your variables seem to be local variables (on the stack) that cease to exist when your function returns.

So would your function end with

return abc;

then you loose everything because you said abc= &def; and def is also a local variable. So yes, you can simply do abc->line= p1; but after the return you have lost abc (which points to def which was a stack variable that then no longer exists) and hence you have also lost abc->line and so you have a memory leak.

So you should expand the code you show us for us to be able to say "Yes you can do that".

The following is an example of a function that is wrong:

struct el{
    char *line;
    struct el *next;
};

struct el *example(void)
{
    struct el *abc, def;    // def is a local variable
    char *p1;
    char buffer[100];
        
    abc = &def;
    gets(buffer);
    p1 = malloc( strlen(buffer) + 1 );
    strcpy( p1, buffer);
    
    abc->line= p1;
    return abc;             // def won't exist anymore after the function returns
}

To fix this error, you should also do abc= malloc(sizeof(struct el)); or pass a pointer variable to the function, for example:

void example2(struct el *abc)
{
    char *p1;
    // ...
    abc->line= p1;
 }

and call as for example:

int main(void)
{
    struct el pqr = {0};
    example2(&pqr);
    // ...
}
Paul Ogilvie
  • 25,048
  • 4
  • 23
  • 41
  • So do i need to use pointers to pointers? @Paul Ogilvie – Andrea Aug 25 '20 at 13:37
  • 3
    @Andrea - No, you do not _need_ to use double pointers to update values of a struct instance. That is not the main message Paul is providing here. He is referring to the fact that you have created local, automatic variables within an area of temporary lifespan. The stack locations(addresses) created while execution flow is within the function will cease to exist upon leaving. – ryyker Aug 25 '20 at 14:13
  • @Paul Ogilvie So let's say that struct el *abc is a parameter passed to the function, when i write abc->line = p1, will this assignment remain also after the return? – Andrea Aug 25 '20 at 18:49
  • @Paul Ogilvie I mean, will abc->line still point to the area allocated inside the function (the one pointed by p1)? – Andrea Aug 25 '20 at 19:05
  • 1
    If the function signature would be for example `void f(struct el *abc)` then `abc->line= p1;` is correct. That is because `abc` no longer points to a local variable. – Paul Ogilvie Aug 26 '20 at 07:39
  • I added an example. – Paul Ogilvie Aug 26 '20 at 08:13
1

How can I make line point to the same string pointed by p1 without allocating the same string twice? Is it enough to write:

abc->line = p1;

Yes, you can do that. abc->line will be pointing to the same memory address as p1.

Or do I also need to free the area pointed by 'p1'?

No, you don't need to free both. You should either use:

free(abc->line);

or:

free(p1);

You only need one of them, as both pointers are pointing to the same memory address, when you free one you free both.

Note that by freeing abc->line, p1 will become a dangling pointer, it will be pointing no nothing meaningful.

Recommended read regarding gets.

anastaciu
  • 23,467
  • 7
  • 28
  • 53
1

First, using gets() is not recommended. A good replacement is fgets().

How can I make 'line' point to the same string pointed by 'p1' without allocating >the same string twice?

Two pointers can both point to the same buffer simply by setting them to point to the address of the buffer. Dynamic memory allocation is not necessary. Given your code:

//the following are created in file global scope
struct el{
    char *line;
    struct el *next;
};

struct el *abc, def; 

char *p1;
char buffer[100];

//The following is performed in local scope within a function, using global variables created above.
p1 = &buffer[0];//assign address of buffer to pointer p1
//same for char *line:
abc->line = &buffer[0];//assign address of buffer to member *line

//both will now accept input using fgets()
fgets(p1, sizeof(buffer), stdin);
fgets(abc->line, sizeof(buffer), stdin);
ryyker
  • 22,849
  • 3
  • 43
  • 87
1

How can I make line point to the same string pointed by p1 without allocating the same string twice? Is it enough to write abc->line = p1?

Yes. abc->line = p1; is correct.

Or do I also need to free the area pointed by p1?

No, not at all. In opposite, If you would do so, line would point to nowhere.

Note that abc->line = p1; only assigned the value of the memory address of the chunk allocated by malloc() from p1 to the structure member line. They both are references to the **same ** memory allocated by malloc().


Never ever use gets(). Reason why is here. It is also removed from the C standard since C11.