Possible memory issue in linked list program [closed]

The well hidden bug.

This allocates strings of length 1.

str=(char*)malloc(sizeof(char));
ch=(char*)malloc(sizeof(char));

This is enough to hold the terminating '\0' but nothing else. You probably want to hold a longer string try.

str = (char*)malloc(sizeof(char) * 500 );
ch  = (char*)malloc(sizeof(char) * 500 );

You should also check to see if they actually worked and returned a good value. One of the major problems with C code is forgetting to check the result of a call to make sure it actually worked.

Scanning. This fails if the input is an empty line. (it will not remove the '\n' on empty lines.

scanf("%[^\n]%*c", str);

Also you should probably put the maximum size of the string encoded into the format specifier.

scanf("%500[^\n]", str);
scanf("%*c");            // Now read and throw away the newline character.
                         // Thus you strip the newline even if the read fails.
                         // PS you may want to check the read worked.

Code Review:

Avoid global variables.

node *ptr, *newnode, *first=NULL, *last=NULL;

There is no need for any of these variables. The variables ptr and newnode should be local varaibles declared inside functions. The variables first and last describe a list. It would be best if you wrapped these up into a structure (as they represent a single entity (a list)). Your code is currently written to support a single list. If you pass this as a parameter to your functions you can support multiple lists in your application (it also makes your code much easier to read.

typedef struct List
{
     node*  first;
     node*  last;
} List;

int main()
{
    List   wordList = { NULL, NULL};
    // STUFF
                //  pass word list as a parameter.
       processword(&wordList, ch); 
}

Indide:

void insertnewword(char *ch)

As mentioned above newnode should be a local function variable (not a global) and you should pass the list you are adding the node onto as the first parameter so that you don't need to keep first and last as global variables.

In this condition we have already asserted that first == last

    first->next=NULL;
    last->next=NULL;

So this double assignment of NULL is redundant. Also When you create the node you have already set it up to be NULL. So there is no point in setting it to NULL again.

On the other side of the else you have the same issue.

    last->next=NULL;            

At this point last points at the new node. Which had its next value set to NULL during construction.

Another trick you could look up is using a sentinel node. Basically you always put one fake node into the list. Then the code for handling insert/delete become much easier to write because you don't need to check if the list is NULL (because there is always at least one node).

typedef struct List
{
     node   sentinel;
     node*  first;
     node*  last;
} List;

void initList(List* list)
{
    list->sentinel = {NULL, -1, NULL};

    list->first = &list->sentinel;
    list->last  = list->first;
}
void destroyList(List* list)
{
    node* loop;
    node* next;
    for(loop = list->head->next;loop != NULL; loop = next)
    {
         next = loop=>next;
         free(loop);
    }
}
void insertnewword(List* list, char* ch)
{
    node*  newnode = (node*)malloc(sizeof(node));
    if (newnode == NULL)
    {  /* ERROR */ exit(0);
    }
    newnode->word  = ch;
    newnode->count = 1;
    newnode->next  = null;

    list->last->next = newnode;
    last->last       = newnode;
}

In:

void processword(char *ch)

As mentioned above ptr should be a local function variable (not a global) and you should pass the list you are adding the node onto as the first parameter so that you don't need to keep first as global variables.

This is not wrong.

for(ptr=first;ptr!=NULL;ptr=ptr->next)          
    if(strcicmp(ptr->word, ch) == 0)
    {}

But I would always put the {} braces around sub statement groups. It becomes too easy for people to accidentally add more code to the for loop without thinking about it and suddenly what you expect to be in the loop is not actually part of the loop. Also it keeps your style consistent if you always have the {} rather than only sometimes having them.

Same comment about {} here.

if(!found)
    insertnewword(ch);  

A lot of poeple use the uneven style.

if(!found) {
    insertnewword(ch);
}

This saves some space. Personally I still use the even style.

if(!found)
{
    insertnewword(ch);
}

But to be honest I am starting to like the uneven style and may change. But for now the matching open and close above each other appeal to me more.