L-value problem

[Pete]

In the following source I need to enter a string into a list. I'm using
dataitem[MAX] and data[MAX] for this, upon exit input string should be
displayed before being free()'ed. Since changing the code from an char
dataitem to dataitem[] I'm getting the error
left operand must be l-value
on the 2 lines in the AddItem() function, the lines are - listpointer ->
dataitem = data; 2 times. The changes I made are the [] in prototypes and
in
main() char data[MAX] and #define MAX 10. Your help is very much
appreciated.

/*Code to insert into nth position in linked list*/
#include <stdio.h>
#include <stdlib.h>
#include "ctype.h"

#define FALSE 0
#define MAX 10

typedef struct listelement { /*structure listelement*/
int position; /*variable to hold postion in string*/
char dataitem[MAX]; /*pointer to char*/
struct listelement *next; /*pointer to next struct item*/

} listelement;

/*Prototypes*/

void Menu(int *choice); /* Menu function */
listelement *AddItem (listelement *listpointer, int posn, char data[]);
/* AddItem function */
void ClearQueue(listelement *listpointer); /* ClearQueue function */
listelement *RemoveItem (listelement *listpointer); /* RemoveItem
function */

/********************main**********************/

main () {
listelement *listpointer;
char data[MAX]; /*variable to take data element*/
int posn, choice; /*variables to take posn & Menu choice*/

listpointer = NULL; /*1st element in list*/
do {
Menu(&choice);
switch(choice) { /*switch procedure for Menu*/
case 1:
fflush(stdin);
printf("Enter data item to add to list\n");
scanf("%s", &data);
printf("enter position in list\n");
scanf("%d", &posn);
listpointer = AddItem(listpointer, posn, data); /* call AddItem
function */
case 2:
break;
}
}while(choice !=2);
ClearQueue(listpointer); /* clear the queue */

} /* End main !!!*/

/*********************Menu*********************/

void Menu(int *choice) {

char local; /* local variable */

printf("\nEnter\t1 to add item,\n\t2 to quit\n");
do {
local = getchar();
if((isdigit (local) == FALSE) && (local != '\n')) {
printf("\nyou must enter an integer.\n");
printf("Enter 1 to add, 2 to quit\n");
}
} while(isdigit((unsigned char) local) == FALSE);
*choice = (int) local - '0';
}

/******************AddItem********************/

listelement *AddItem (listelement *listpointer, int posn, char data[])
{
listelement *lp = listpointer;

if (listpointer != NULL) {
while (listpointer -> next);
listpointer -> next = (struct listelement *) malloc(sizeof(listelement));
listpointer = listpointer -> next;
listpointer -> next = NULL;
listpointer -> dataitem = data;
listpointer -> position = posn;
return lp;
}
else {
listpointer = (struct listelement *) malloc (sizeof (listelement));
listpointer -> next = NULL;
listpointer -> dataitem = data;
listpointer -> position = posn;
return listpointer;
}

}

/*****************ClearQueue*********************/

void ClearQueue(listelement *listpointer) {
while (listpointer != NULL) {
listpointer = RemoveItem (listpointer);
}

}

/*****************RemoveItem**********************/

listelement *RemoveItem (listelement *listpointer) {

listelement *tempp; /* temporary */
printf ("Element removed is %s\n", listpointer -> dataitem);
printf ("Position of element removed is %d\n", listpointer -> position);
tempp = listpointer -> next;
free (listpointer);
return tempp;

}

 

[Ian Collins]

In the following source I need to enter a string into a list. I'm using
dataitem[MAX] and data[MAX] for this, upon exit input string should be
displayed before being free()'ed. Since changing the code from an char
dataitem to dataitem[] I'm getting the error
left operand must be l-value
on the 2 lines in the AddItem() function, the lines are - listpointer ->
dataitem = data; 2 times. The changes I made are the [] in prototypes and
in
main() char data[MAX] and #define MAX 10. Your help is very much
appreciated.

/*Code to insert into nth position in linked list*/
#include<stdio.h>
#include<stdlib.h>
#include "ctype.h"

#define FALSE 0
#define MAX 10

typedef struct listelement { /*structure listelement*/
int position; /*variable to hold postion in string*/
char dataitem[MAX]; /*pointer to char*/
struct listelement *next; /*pointer to next struct item*/

Those comments are pointless and in the case of dataitem, plain wrong.

} listelement;

/*Prototypes*/

void Menu(int *choice); /* Menu function */
listelement *AddItem (listelement *listpointer, int posn, char data[]);
/* AddItem function */
void ClearQueue(listelement *listpointer); /* ClearQueue function */
listelement *RemoveItem (listelement *listpointer); /* RemoveItem
function */

/********************main**********************/

main () {
listelement *listpointer;
char data[MAX]; /*variable to take data element*/
int posn, choice; /*variables to take posn& Menu choice*/

Again, pointless comments.

listelement *AddItem (listelement *listpointer, int posn, char data[])
{
listelement *lp = listpointer;

if (listpointer != NULL) {
while (listpointer -> next);
listpointer -> next = (struct listelement *) malloc(sizeof(listelement));

Drop the cast.

listpointer = listpointer -> next;
listpointer -> next = NULL;

The spaces around the -> are confusing.

listpointer -> dataitem = data;

This shouldn't compile.

listpointer -> position = posn;
return lp;
}
else {
listpointer = (struct listelement *) malloc (sizeof (listelement));
listpointer -> next = NULL;
listpointer -> dataitem = data;

Neither should this.

 

[Ian Collins]

This shouldn't compile.

I forgot to say why - you have to copy (strcpy) the data, you can't
assign arrays.

 

[Ben Bacarisse]

In the following source I need to enter a string into a list. I'm using
dataitem[MAX] and data[MAX] for this, upon exit input string should be
displayed before being free()'ed. Since changing the code from an char
dataitem to dataitem[] I'm getting the error
left operand must be l-value
on the 2 lines in the AddItem() function, the lines are - listpointer ->
dataitem = data; 2 times. The changes I made are the [] in prototypes and
in
main() char data[MAX] and #define MAX 10. Your help is very much
appreciated.

This is because C does not permit you to assign arrays -- at least not
directly as you are trying to do. You have to copy the chars from the
argument of AddItem to the listpointer->dataitem array. Loop up strlen
and memcpy in you C reference. You can also do it using strncat.

/*Code to insert into nth position in linked list*/
#include <stdio.h>
#include <stdlib.h>
#include "ctype.h"

#define FALSE 0
#define MAX 10

typedef struct listelement { /*structure listelement*/
int position; /*variable to hold postion in string*/
char dataitem[MAX]; /*pointer to char*/
struct listelement *next; /*pointer to next struct item*/
} listelement;

There is a trick that is sometimes used. Arrays are copied when they are
inside a struct so one can use a dummy instance of 'listelement' and
read into that:

listelement input;
scanf("%9s", input->dataitem);
AddItem(/*...*/, &input);

Inside AddItem you copy the dummy item into the newly allocated node (by
a single structure assignment) and then set the other fields. It's
something of a kludge, but it helps to know these things if only so you
can decide not to use it!

/*Prototypes*/

void Menu(int *choice); /* Menu function */
listelement *AddItem (listelement *listpointer, int posn, char data[]);
/* AddItem function */
void ClearQueue(listelement *listpointer); /* ClearQueue function */
listelement *RemoveItem (listelement *listpointer); /* RemoveItem
function */

/********************main**********************/

main () {
listelement *listpointer;
char data[MAX]; /*variable to take data element*/
int posn, choice; /*variables to take posn & Menu choice*/

listpointer = NULL; /*1st element in list*/
do {
Menu(&choice);
switch(choice) { /*switch procedure for Menu*/
case 1:
fflush(stdin);
printf("Enter data item to add to list\n");
scanf("%s", &data);

You don't need the & and you should limit the number of characters you
read.

printf("enter position in list\n");
scanf("%d", &posn);
listpointer = AddItem(listpointer, posn, data); /* call AddItem
function */
case 2:
break;
}
}while(choice !=2);
ClearQueue(listpointer); /* clear the queue */

} /* End main !!!*/

/*********************Menu*********************/

void Menu(int *choice) {

char local; /* local variable */

printf("\nEnter\t1 to add item,\n\t2 to quit\n");
do {
local = getchar();
if((isdigit (local) == FALSE) && (local != '\n')) {

It's not good style to compare the result of a predicate to FALSE (or
indeed to TRUE). Just write !isdigit(local).

[Technically, you need to write !isdigit((unsigend char)local) -- but
that is point so often overlooked that it hardly matters any more.]

printf("\nyou must enter an integer.\n");
printf("Enter 1 to add, 2 to quit\n");
}
} while(isdigit((unsigned char) local) == FALSE);

Ah! You do it here. Well done.

*choice = (int) local - '0';

The cast does nothing.

}

/******************AddItem********************/

listelement *AddItem (listelement *listpointer, int posn, char data[])
{
listelement *lp = listpointer;

if (listpointer != NULL) {
while (listpointer -> next);

Eh? Is this code tested?

listpointer -> next = (struct listelement *) malloc(sizeof(listelement));

The cast is usually considered bad style. You should test the result
from malloc, too.

<snip code I had not time to look over, sorry>

 

[Pete]

I forgot to say why - you have to copy (strcpy) the data, you can't
assign arrays.

Hi Ian

I think you were right, except that strcopy wont work in this case
because I've got char arrays, not strings (char*). But I found a memcopy
function which works here:

memcpy((void*) &((listpointer -> dataitem)[0]), (void*) &(data[0]),
strlen((char*) &(data[0])));

 

[Ben Bacarisse]

Hi Ian

I think you were right, except that strcopy wont work in this case
because I've got char arrays, not strings (char*).

strcpy will work better than the code you used. (It will be better
because it will produce a string which your code does not because no
null charcter is copied or written to the destination array.) strcpy
can even be used safely if you check the string length before you copy.

But I found a memcopy
function which works here:

memcpy((void*) &((listpointer -> dataitem)[0]), (void*) &(data[0]),
strlen((char*) &(data[0])));

This rather complex line is the same as:

memcpy(listpointer->dataitem, data, strlen(data));

but it's wrong. First, you need to be sure you copy no more characters
than will fit and, second, you need to ensure that the result is
null-terminated or it won't be a string. You can do both at once like
this:

memcpy(listpointer->dataitem, data, sizeof listpointer->dataitem);
listpointer->dataitem[sizeof listpointer->dataitem - 1] = 0;

Alternatively you can use strncat

listpointer->dataitem[0] = 0; /* start with an empty string */
strncat(listpointer->dataitem, data, sizeof listpointer->dataitem - 1);