linked list built but not working properly

Question

I have numbers in a linked list which i am supposed to convert to char* and return .

Here is is the function where the problem probably exists.

char* int_str(struct Node** head,char* result) //head is pointer to singly linked list
{
  struct Node* temp = *head;
  char* string1="";
  char* str;
  while(temp != NULL)
  {
    string1=myitoa(temp->data,string1); // myitoa() works fine 
    str=(char*)malloc(1+strlen(string1));
    strcpy(str,string1);
    strcat(result,str);
    temp=temp->next;
  }
  return result;
}

The last call to temp->data always ends up having unknown garbage values . (The linked list is built correctly as printing the linked list works properly.)

Example : the linked list is 1->2->3

The last call to temp->data in the function gives 50(some garbage value) ie 1->2->50

while in the main function the list correctly gives 1->2->3

The last variable ends up as garbage in this function but shows correctly in main function why ?

you have a memory leak with str. You don't free the allocated block. Why not simply `strcat(result, string1)` — chmike, Mar 05 '15 at 13:34
myitoa() converts the int into a sequence of chars I suppose. But where does it store these chars ? — chmike, Mar 05 '15 at 13:36

score 2 · Answer 1 · edited May 23 '17 at 12:12

2

char* string1;

This declares a pointer but does not allocate a buffer for the string. It's a bit confusing that this uninitialized pointer is passed to myitoa, because myitoa can not do anything with it without causing undefined behaviour.

This line always allocates 5 bytes on 32 bit systems:

str=(char*)malloc(1+sizeof(string1));

sizeof(string1) is the size of the pointer, not of the string. Use

str=(char*)malloc(1+strlen(string1));

instead, or, even better, use

str=malloc(1+strlen(string1));

because the result of malloc should not be casted.

edited May 23 '17 at 12:12

Community

1
1

answered Mar 05 '15 at 13:34

alain

11,939
2
31
51

i do not understand the whats the problem with string1 in myatoi() i have used print statements and it does work fine.But thanks for the link learnt something new today :) – solinvictus Mar 05 '15 at 14:09
You're welcome :-). `string1` points to a random address, because there is no statement that sets the pointer (`string1 = .....`). If that random adress is read or written to, it's pure luck that the program does not crash. – alain Mar 05 '15 at 14:21

score 1 · Accepted Answer · answered Mar 05 '15 at 13:38

1

This piece of code will most probably do the job. There is no need to malloc. str is defined as an array of 20 chars. This is where the number in ASCII is stored.

char* int_str(struct Node** head,char* result) //head is pointer to singly linked list
{
    struct Node* temp = *head;
    char str[20];
    while(temp != NULL)
    {
        strcat(result, myitoa(temp->data, str));
        temp=temp->next;
    }
    return result;
}

answered Mar 05 '15 at 13:38

chmike

20,922
21
83
106

Thanks i was hell bent on using malloc my ego got the better of me.The above code would obviously work fine.But how do i get the code in question to work ? – solinvictus Mar 05 '15 at 13:48
First change `char *string1 = "";` into `char string1[20];`. Second add a `free(str);` after `temp=temp->next;`. This will make your code work. `char string1[20];` provides the storage needed by myitoa(). – chmike Mar 05 '15 at 14:17
Yes i forgot to de-allocate memory here .Why does the array declaration work and not char* . – solinvictus Mar 05 '15 at 14:22
1

Because `char *string1 = "";` declares `string1` as a pointer on the constant string "". There is no room in an empty string to store the integer number in ASCII and you are not supposed to modify a constant string. – chmike Mar 05 '15 at 14:27
Ah ha ! now it all makes sense.Thank you ! that was actually basic . I am learning C with python as my first language.C Makes me wonder who would use such cryptic language any longer :) but i guess every language has its own use . – solinvictus Mar 05 '15 at 14:36
I understand. With C you have to manage storage yourself. Your code can be much more efficient than Python this way. But this is indeed not trivial and error prone. – chmike Mar 05 '15 at 14:38

coolVick · Answer 3 · 2015-03-05T13:57:08.987

0

I don't think that the problem lies in string manipulations. Most probably it is related to the creation of linked list. Try this:

First of all, try printing the value of temp->data in the loop. Possibly it should be garbage (in which case the list creation contains problem). If the printed value is correct, then check if the "result" string which is being returned, holds the same value or not. If not, then the problem is in myitoa function.

Here I am assuming that myitoa is allocating memory to string1.(if not, then you have to allocate some memory to it either before passing it to function or inside myitoa.

edited Mar 05 '15 at 13:57

answered Mar 05 '15 at 13:48

coolVick

538
3
7

From the question :The linked list is built correctly as printing the linked list works properly. – solinvictus Mar 05 '15 at 14:03
From the answer:Here I am assuming that myitoa is allocating memory to string1.(if not, then you have to allocate some memory to it either before passing it to function or inside myitoa.) – coolVick Mar 05 '15 at 14:27

linked list built but not working properly

3 Answers3