Pointer to char

Question

Here's a snippet:

void addproductInterface(Tsklep **head){
    char* name = (char*)malloc(sizeof(char)*100);
    double price;
    do{
        printf("Name: ");
        scanf("%s[^\n]", name);
        fflush(stdin);
        printf("\nPrice: ");
        scanf("%lf", &price);
        fflush(stdin);
        addProduct(&(*head), name, price);
    } while(prompt("Do you want to add another one?"));

it works, but after I add another product, it changes the previous one (and the previous ones) to this name. It seems, that I pass the same pointer every time and I just change an array (when I add another product) it points to. do I understand it correctly? Do you have any ideas how to fix it?

can you please provide `addProduct` code – Chirag Desai May 31 '13 at 09:07 — Chirag Desai, May 31 '13 at 09:07

score 3 · Answer 1 · edited May 23 '17 at 11:43

It sounds like what you describe, yes. It's hard to tell for sure without seeing the code for addProduct(), but that would be the place to allocate new memory.

You should use a temporary, automatic (on stack), buffer for the input, then do the permanent allocation when you store the record, in addProduct():

do{
    char name[64];
    double price;

    printf("Name: ");
    scanf("%63s", name);
    fflush(stdin);
    printf("\nPrice: ");
    scanf("%lf", &price);
    fflush(stdin);
    addProduct(&(*head), name, price);
} while(prompt("Do you want to add another one?"));

You should also error-check the scanf() calls, they can fail if given unexpected input.

Also, don't cast the return value of malloc() in C.

score 2 · Answer 2 · answered May 31 '13 at 09:07

2

You allocate the name only once, at the beginning of the function, so you overwrite the content on each loop execution. Move the allocation in do-while cycle.

answered May 31 '13 at 09:07

Cătălin Pitiș

14,123
2
39
62

score 1 · Accepted Answer · answered May 31 '13 at 09:07

1

Move the allocation of what name points to into the loop.

answered May 31 '13 at 09:07

alk

69,737
10
105
255

thank you, now working.. what about memory leak? Should I use free? – tomdavies May 31 '13 at 09:09
@TomDavies92: This depends on what `addProduct()` does. Does it copy the content of `name`? – alk May 31 '13 at 09:12
it's just setting the new element and adds to the list, product->name = name; // name passed from addProduct(&(*head), name, price); – tomdavies May 31 '13 at 09:16
oh, I used crtdbg.h to detect memory leaks, there are in this place (where I use malloc to the name). How to fix it? I tried using free(), but it's not working ;/ – tomdavies May 31 '13 at 09:19
@TomDavies92: So it would be bad idea to free the memory allocated in the while-loop as you keep arefernce to it. – alk May 31 '13 at 09:20
Fixed the memory leak -> I copy name in addProduct, then I can use free() in do..while(). I updated my cleaning function, I added free(product->name) and it's working now :) – tomdavies May 31 '13 at 09:45

score 1 · Answer 4 · answered May 31 '13 at 09:12

1

Move the line char* name = (char*)malloc(sizeof(char)*100); inside the do while loop as follows,

do{
    char* name = (char*)malloc(sizeof(char)*100);
    printf("Name: ");
    scanf("%s[^\n]", name);
    fflush(stdin);
    printf("\nPrice: ");
    scanf("%lf", &price);
    fflush(stdin);
    addProduct(&(*head), name, price);
} while(prompt("Do you want to add another one?"));

answered May 31 '13 at 09:12

Deepu

7,592
4
25
47

1

-1, this can't possibly be right. If `addProduct()` is retaining the `name` pointer as it seems, this will `free()` the memory making the stored pointer invalid. – unwind May 31 '13 at 09:13
This is a good answer, but you should remove the call to `free`, it's harmful here: From the problem, it's clear `addProduct` does not duplicate the buffer, but instead keeps a pointer to it inside each `Tsklep` structure. – Medinoc May 31 '13 at 09:14
and btw. I write in strict C - I can't define variables wherever I want :/ – tomdavies May 31 '13 at 09:15
Is it still wrong? If you want you could edit my answer and remove the down votee :) – Deepu May 31 '13 at 09:30

score 1 · Answer 5 · answered May 31 '13 at 09:12

1

You should move your allocation inside the loop.

(by the way, your call to scanf is susceptible to overflow name since you don't limit the size.)

answered May 31 '13 at 09:12

Medinoc

6,577
20
42

Pointer to char

5 Answers5