C and string handling

Question

FULL CODE http://pastebin.com/6bdVTyPt My tree code was perfectly working until i found out i needed to validate the id that its not text so it had to be string insertion function string compare of 90 and 129 returns 8 tried to use (atoi) and compare as integers does not work any help appreciated
thank you heres the insert function using atoi instead of strcomp http://pastebin.com/yeuktyAF still not working insertion function

   struct node * insert2(struct node *root, char x[],char id[])
 {
if(!root)
{
    root=(struct node*)malloc(sizeof(struct node));
    free( root->data );
    free( root->id );// free previously allocated memory, if any
    root->data = strdup( x ); // malloc and copy
    root->id=strdup(id);
    root->left = NULL;
    root->right = NULL;
    //   printf("1\n");
    return(root);
}
printf("string comp %d of %s of %s\n",strcmp(root->id,id),root->id,id);
if((strcmp(root->id,id))>0){
    root->left = insert(root->left,x,id);
    printf("go left\n");
}
else
{
    if(strcmp(root->id,id)<0){
        printf("go right\n");
        root->right = insert(root->right,x,id);}
}
return(root);
}

score 1 · Answer 1 · answered May 15 '13 at 08:34

1

The line

root=(struct node*)malloc(sizeof(struct node));

allocates memory for root but doesn't initialise it. This means that the following lines

free( root->data );
free( root->id );

attempt to free uninitialised (so unpredictable) pointers. This will almost certainly crash.

Since you've only just allocated root there can't possibly be any previous values in data or id to free. This means you could simplify these three lines to

root=malloc(sizeof(*root));

answered May 15 '13 at 08:34

simonc

41,632
12
85
103

i am new to memory allocation and this was a tip from somebody here on stackflow because it wasnt working – Mahmoud ElMarakshy May 15 '13 at 09:28
I'm afraid the suggestion to add the call to `free` was very poor advice. You must do this if you have previously allocated memory for `root->data`; you **must not** do this for the uninitialised pointer you have in this case. – simonc May 15 '13 at 09:32
do u have any idea how can i compare numbers in strings? – Mahmoud ElMarakshy May 15 '13 at 10:29
If your string only contains digits, why not change it to be `int` instead? If you really need to store it as a string, you can use [atoi](http://linux.die.net/man/3/atoi) to convert this into an `int` inside `insert2` – simonc May 15 '13 at 10:41
http://pastebin.com/yeuktyAF @simonc try to do that not working :( sorry for ur time – Mahmoud ElMarakshy May 15 '13 at 11:43
I can't see any issues with that code. Its possible the issue could lie in the calling code. If you're still having problems, it might be better to create a small example program that demonstrates the problem and post it as a new question. – simonc May 15 '13 at 16:30

score 0 · Answer 2 · edited May 23 '17 at 11:49

0

You can't compare numerical strings using strcmp(). You should store your ID:s as integers, if that's what they are, so you can compare them directly.

That also has the benefit of less complexity, since integers are fixed size (assuming unsigned long is long enough) you won't need to use strdup().

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

edited May 23 '17 at 11:49

Community

1
1

answered May 15 '13 at 08:58

unwind

391,730
64
469
606

yes okey the thing is i neeed to validate the id using `code` int validid(char a[]){int c,flag=0; for(c=0; (a[c])!='\0'; c++) { if(isalpha(a[c])) flag=1; } if (flag==1) { return 0; } else return 1; } – Mahmoud ElMarakshy May 15 '13 at 09:26

C and string handling

2 Answers2

Linked