Statement with no effect with pointers

Question

I have 2 structs using pointers to form a linked list and I'm creating operations that work over these types:

typedef struct hashtag { 
    char *text;
    int count;
} *Item;

typedef struct node {
    Item item;
    struct node *next;
} *link;

I'm having a couple issues with pointers all on the same function.

/* adds hashtag (Item Type) to linked list (Link Type) */

void add_hashtag(char *word){
    int search_result, i;
    for (i = 0; i < strlen(word); i++)

     /* converts word to all lowercase before adding */

    token[i]=tolower(*(token + i));
    Item buffer = (Item) malloc(sizeof(struct hashtag));
    strcpy(buffer->text,word);

    /* Runs through the list to see if the hashtag was already added */

    search_result = (search_hashtag(head, buffer));
    if (search_result!=NULL){

        /* Increase count (occurrence) of hashtag if it already exists (returns link type or NULL if not found) */

        increase_hashtag_count(search_result);
    }

    /* Create new struct type hashtag */
    new_hashtag(buffer);

}

warning: assignment makes integer from pointer without a cast [-Wint-conversion] search_result = (search_hashtag(head, buffer));

warning: comparison between pointer and integer if (search_result!=NULL){

The tolower() function and search_result() are not working with pointers correctly and I'm having trouble debugging this.

edit: tolower() fixed, I misread the documentation

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, May 14 '16 at 09:22
`int search_result` but It has been required that a pointer. `strcpy(buffer->text,word);` : `buffer->text` does not allocated. — BLUEPIXY, May 14 '16 at 09:29

score 1 · Answer 1 · answered May 14 '16 at 10:31

There are several errors in the function.

First of all it is not clear what is variable tokenand where and how it is declared, used, and set

for (i = 0; i < strlen(word); i++)

 /* converts word to all lowercase before adding */

token[i]=tolower(*(token + i));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

It is desirable to check that a call to malloc was successfull. For example

Item buffer = (Item) malloc(sizeof(struct hashtag));
if ( buffer )
{
    //...other stuff
}

The memory for buffer->text was not allocated. So this statement

strcpy(buffer->text,word);

has undefined behaviour.

Variable search_result is declared as having type int. You should to compare it with an integer instead of a pointer

search_result = (search_hashtag(head, buffer));
if (search_result!=NULL){
^^^^^^^^^^^^^^^^^^^^^^^^^

It seems that if hashtag already exists you should not add a new hashtag to the list in this statement

new_hashtag(buffer);

It is enough to increase the count (occurrence) of the existed hashtag.

Very good help, thanks a lot! Regarding the buffer->text allocation can you elucidate me a bit more? By using 'Item buffer = (Item) malloc(sizeof(struct hashtag));' am I not allocating memory for both the Item.count and Item.text variables? Also regarding the search_hashtag(), it returns NULL if it's not already found. So != NULL = new_hashtag(); But Im not so sure as how to call the function properly with pointers — spacing, May 14 '16 at 11:15
@spacing struct hashtag has two data members count and pointer text that will point to the string word as I understand. So you need to allocate memory for a copy of word. For example buffer->text = malloc( strlen( word ) + 1 ); . As for the function serach_hashtag then you should determine what return type it has. — Vlad from Moscow, May 14 '16 at 17:14

Sourav Ghosh · Answer 2 · 2016-05-14T09:34:25.170

0

As per the man page of tolower(),

The value returned is that of the converted letter, or c if the conversion was not possible.

That means, it does not change the passed argument, it returns the result as the function call return value and in your case, you are ignoring the return value, making the whole function call pointless.

So, you need to have another array to store the return value of tolower() to get the converted lowercase string.

That said, for the second case, you have defined search_result as int but later, you're trying to compare it against NULL, which is a null-pointer constant. You need to correct the data type accordingly.

edited May 14 '16 at 09:34

answered May 14 '16 at 09:25

Sourav Ghosh

133,132
16
183
261

Yeah I completely misread the docs somehow, I've fixed that issues now but the other issues with search results still persist – spacing May 14 '16 at 09:32
@spacing: so what issues remain? You only say that they 'are not working correctly'. – Jongware May 14 '16 at 10:13
@RadLexus (search_hashtag)/search_result() is not working properly with the same errors of the main post – spacing May 14 '16 at 10:18

Statement with no effect with pointers

2 Answers2