invalid lvalue in assignment

[Chad]

Given the following...

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define MAX 100

void choparray(char **s)
{
int i;
int j;
char nrows;
char ncolumns;

ncolumns = 10;
nrows = 10;

char **buf = malloc(nrows * sizeof(char *));
for(i = 0; i < nrows; i++)
buf = malloc(ncolumns * sizeof(char));

j = strlen(*s) -1;
strcpy(*buf, *s);
*buf+j = '\0';
}

int main(void)
{
char *stuff[]= {"test", "this"};
choparray(stuff);

printf("after: %s\n", *stuff);

return 0;
}

I get "invalid lvalue in assignment" when I try to compile it....

$ gcc -g strip.c -o strip
strip.c: In function ‘choparray’:
strip.c:24: error: invalid lvalue in assignment

How come

*buf+j = '\0';

Wouldn't be equivalent to

buf[j] = '\0';

in this case

 

[dj3vande]

I get "invalid lvalue in assignment" when I try to compile it....

$ gcc -g strip.c -o strip
strip.c: In function choparray:
strip.c:24: error: invalid lvalue in assignment

How come

*buf+j = '\0';

Wouldn't be equivalent to

buf[j] = '\0';

in this case

Because '*buf+j' is equivalent to '(*buf)+j'.
Perhaps you meant '*(buf+j)'?
(But it seems to me that 'buf[j]' would be easier to read anyways.)


dave

 

[Keith Thompson]

Given the following...
[...]
int j; [...]
char **buf = malloc(nrows * sizeof(char *)); [...]
*buf+j = '\0'; [...]
I get "invalid lvalue in assignment" when I try to compile it....

$ gcc -g strip.c -o strip
strip.c: In function `choparray':
strip.c:24: error: invalid lvalue in assignment

How come

*buf+j = '\0';

Wouldn't be equivalent to

buf[j] = '\0';

in this case

No.

buf[j] is equivalent to *(buf+j).

*buf+j is equivalent to (*buf) + j, which is not an lvalue.

Two question:

1. Why not just write buf[j]?

2. buf[j] is of type char*. You *can* assign '\0' to it, but it's
being taken as a null pointer constant. (Try changing '\0' to, say,
'X'; you should get a diagnostic message.) Either you should be using
NULL rather than '\0', or you should be assigning '\0' to something
other than buf[j]. I haven't examined the logic of your code enough
to know which.

 

[Ian Collins]

Given the following...

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define MAX 100

void choparray(char **s)
{
int i;
int j;
char nrows;
char ncolumns;

ncolumns = 10;
nrows = 10;

char **buf = malloc(nrows * sizeof(char *));
for(i = 0; i < nrows; i++)
buf = malloc(ncolumns * sizeof(char));

j = strlen(*s) -1;
strcpy(*buf, *s);
*buf+j = '\0';
}

*buf+j is an rvalue (in this case, an address), you can't assign a value
to it.

If you break the expression down:

char *p = *buf+j;
p = '\0';

Or

*(*buf+j) = '\0';

You can see it probably isn't what you wanted to do.

 

[Chad]

Given the following...
[...]
int j; [...]
char **buf = malloc(nrows * sizeof(char *)); [...]
*buf+j = '\0'; [...]
I get "invalid lvalue in assignment" when I try to compile it....

$ gcc -g strip.c -o strip
strip.c: In function `choparray':
strip.c:24: error: invalid lvalue in assignment

How come

*buf+j = '\0';

Wouldn't be equivalent to

buf[j] = '\0';

in this case

No.

buf[j] is equivalent to *(buf+j).

*buf+j is equivalent to (*buf) + j, which is not an lvalue.

Two question:

1. Why not just write buf[j]?

2. buf[j] is of type char*. You *can* assign '\0' to it, but it's
being taken as a null pointer constant. (Try changing '\0' to, say,
'X'; you should get a diagnostic message.) Either you should be using
NULL rather than '\0', or you should be assigning '\0' to something
other than buf[j]. I haven't examined the logic of your code enough
to know which.

--

When I do the changes

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

void choparray(char **s)
{
int i;
int j;
char nrows;
char ncolumns;

ncolumns = 10;
nrows = 10;


char **buf = malloc(nrows * sizeof(char *));
for(i = 0; i < nrows; i++)
buf = malloc(ncolumns * sizeof(char));

j = strlen(*s) -1;
strcpy(*buf, *s);
buf[j] = NULL;
printf("%s\n", *buf);
}

int main(void)
{
char *stuff[]= {"test", "this"};
choparray(stuff);

printf("after: %s\n", *stuff);

return 0;
}

I get the following...

[cdalten@localhost oakland]$ gcc -g mall.c -o mall
[cdalten@localhost oakland]$ ./mall
test
after: test


I was expecting to see
tes
after: test

 

[Default User]

Chad wrote:

When I do the changes

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

void choparray(char **s)
{
int i;
int j;
char nrows;
char ncolumns;

ncolumns = 10;
nrows = 10;

You should save some space and make this:

char nrows = 10;
char ncolumns = 10;

char **buf = malloc(nrows * sizeof(char *));
for(i = 0; i < nrows; i++)
buf = malloc(ncolumns * sizeof(char));

j = strlen(*s) -1;
strcpy(*buf, *s);
buf[j] = NULL;

This is not right. What you are doing is setting one of the pointers
you allocated to NULL, leaking memory. You got unlucky because
strlen(*s) happens to be smaller than nrows, so it just silently did
something other than what you wanted.

To chop off a character, you need a second dereference.

buf[0][j] = '\0';

Or something like that.

I'm pretty sure people told you not to use NULL as a character
constant. It's a null pointer constant. If it happens to be defined as
0 or the like, it will work. If it's (void*)0, it won't. If they
didn't, then "don't use NULL as a character constant."




Brian

 

[Keith Thompson]

You should save some space and make this:

char nrows = 10;
char ncolumns = 10;

Better, since you never modify them, and since they don't represent
character values:

const int nrows = 10;
const int ncolums = 10;

(You *might* save a few bytes by declaring them as char rather than
int, but you're likely to pay for it in larger code and the inability
to use values bigger than CHAR_MAX, which can be as small as 127.)

[...]

 

[Richard Tobin]

Not true. buf[j] is equivalent to (*((buf)+(j))).

No. (*((buf)+(j))) may cause a translation limit to be exceeded
(e.g. line length) when buf[j] doesn't.

-- Richard

 

[Default User]

Better, since you never modify them, and since they don't represent
character values:

const int nrows = 10;
const int ncolums = 10;

Excellent point(s).



Brian

 

[Default User]

Chad wrote:

void choparray(char **s)
{
int i;
int j;
char nrows;
char ncolumns;

ncolumns = 10;
nrows = 10;


char **buf = malloc(nrows * sizeof(char *));
for(i = 0; i < nrows; i++)
buf = malloc(ncolumns * sizeof(char));

j = strlen(*s) -1;
strcpy(*buf, *s);
buf[j] = NULL;
printf("%s\n", *buf);
}

Taking another look at this, there are more problems that I noticed at
first.

1. You pass in a pointer to pointer, but no dimensions. I suppose it's
supposed to be an array of strings, based on main(). However, there's
no way of associating nrows and ncolumns with that. What does 10x10
have to do with anything? Certainly not the array of two strings of
length four you passed in.

2. You allocate all that memory, then leak it at the end. You should be
returning it or something. If, based on the name of the function,
you're planning to lop a character off the end of strings, then you
don't need to allocate anything. Explain the goal here.




Brian

 

[Chad]

void choparray(char **s)
{
int i;
int j;
char nrows;
char ncolumns;

ncolumns = 10;
nrows = 10;

char **buf = malloc(nrows * sizeof(char *));
for(i = 0; i < nrows; i++)
buf = malloc(ncolumns * sizeof(char));

j = strlen(*s) -1;
strcpy(*buf, *s);
buf[j] = NULL;
printf("%s\n", *buf);
}

Taking another look at this, there are more problems that I noticed at
first.

1. You pass in a pointer to pointer, but no dimensions. I suppose it's
supposed to be an array of strings, based on main(). However, there's
no way of associating nrows and ncolumns with that. What does 10x10
have to do with anything? Certainly not the array of two strings of
length four you passed in.

I know the nrow and ncolumns should be passed to choparray(). But
quite frankly, it sort of hurt when I thought about how to actually do
it.

2. You allocate all that memory, then leak it at the end. You should be
returning it or something. If, based on the name of the function,
you're planning to lop a character off the end of strings, then you
don't need to allocate anything. Explain the goal here.

I was too lazy to use free() and there is no goal here.

 

[Default User]

I know the nrow and ncolumns should be passed to choparray(). But
quite frankly, it sort of hurt when I thought about how to actually do
it.

void choparray(char **s, int nrows, int ncolumns);

Then use those just like you did. To call:

choparray(stuff, 2, 5); /* or whatever */

I still have doubts about this whole thing. We'll see what the next
paragraph brings. I'm atingle.

I was too lazy to use free() and there is no goal here.

What a letdown.

What is the purpose of choparray()? Right now all it does (if it were
working) is print out one string minus its last character. You could do
that a whole lot easier:

size_t len = strlen(s[0]);
int i;

for (i = 0; i < len-1; i++)
{
putchar(s[0]);
}


Presumably that's not it, so what ARE you trying to achieve with the
function? It LOOKS like you want to remove the last character, that's
normally what a chop function does. Why are you making a copy of it? Do
you not want to change the original array?

Explain the purpose of the function. Presumably it has a purpose, even
if only an academic one. What are the requirements of the function? Why
do you pass in what you do? What should be the overall results of the
function after it runs? These are things you decide before you ever
write a line of code.

If you don't have a clear idea of what the function should do, how will
you ever know if it's doing that?




Brian

 

[Chad]

I know the nrow and ncolumns should be passed to choparray(). But
quite frankly, it sort of hurt when I thought about how to actually do
it.

void choparray(char **s, int nrows, int ncolumns);

Then use those just like you did. To call:

choparray(stuff, 2, 5); /* or whatever */

I still have doubts about this whole thing. We'll see what the next
paragraph brings. I'm atingle.

I was too lazy to use free() and there is no goal here.

What a letdown.

What is the purpose of choparray()? Right now all it does (if it were
working) is print out one string minus its last character. You could do
that a whole lot easier:

size_t len = strlen(s[0]);
int i;

for (i = 0; i < len-1; i++)
{
putchar(s[0]);

}

Presumably that's not it, so what ARE you trying to achieve with the
function? It LOOKS like you want to remove the last character, that's
normally what a chop function does. Why are you making a copy of it? Do
you not want to change the original array?

Explain the purpose of the function. Presumably it has a purpose, even
if only an academic one. What are the requirements of the function? Why
do you pass in what you do? What should be the overall results of the
function after it runs? These are things you decide before you ever
write a line of code.

If you don't have a clear idea of what the function should do, how will
you ever know if it's doing that?

The whole little program stemed from the 'pointer Access violation
thread' on this forum found at the following URL...

http://groups.google.com/group/comp.lang.c/browse_thread/thread/596801078c7d57d9?hl=en#

Basically the original poster wanted to remove the last character in
each array element. He never specified if he wanted to modify the
array in place. Anhow, it sort of looked like an interesting excercise
at the time. Anyhow, here is what I came up with...

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

void choparray(char **s, int nrows)
{
int i;
size_t j;

char **buf = malloc(nrows * sizeof(char *));
for(i = 0; i < nrows; i++) {
j = strlen(*s) -1;
buf = malloc(j * sizeof(char));
strcpy(*(buf + i), *s);
buf[j] = '\0';
*(s++); /*s[something]*/
}

for (i = 0; i < nrows; i ++) {
printf("%s\n", *(buf + i));
}

for(i = 0; i < nrows; i++)
free((void *)buf);
free((void *)buf);
}

int main(void)
{
int number;

char *stuff[]= {"test", "this", "input", "data"};
number = (sizeof stuff/sizeof stuff[0]);

choparray(stuff, number);

return 0;
}
[cdalten@localhost oakland]$ gcc -g -Wall mall.c -o mall
mall.c: In function ‘choparray’:
mall.c:16: warning: value computed is not used
[cdalten@localhost oakland]$ ./mall
tes
thi
inpu
dat
[cdalten@localhost oakland]$

 

[Default User]

Chad wrote:

Basically the original poster wanted to remove the last character in
each array element. He never specified if he wanted to modify the
array in place. Anhow, it sort of looked like an interesting excercise
at the time.

That fine, but what do you want it to do? Change it in place, or return
a new, modified, array? Right now you have something that's neither. By
the way, if you do modify it, you don't want to be passing in those
string literals. Undefined Behavior and all that.





Brian

 

[Chad]

That fine, but what do you want it to do? Change it in place, or return
a new, modified, array? Right now you have something that's neither. By
the way, if you do modify it, you don't want to be passing in those
string literals. Undefined Behavior and all that.

Brian

I think changing it place would be pretty interesting. However, I
really have no idea how to go about doing it. The closest thing that I
can came up is modifying

char *stuff[]= {"test", "this", "input", "data"};

to something like

char stuff[][SOME_MADE_UP_NUMBER] = {"test", "this", "input", "data"};

Then then passing this to choparray(). And yes, I know that I forgot
to check malloc() for failure. Bad me.

 

[Spiros Bousbouras]

The whole little program stemed from the 'pointer Access violation
thread' on this forum found at the following URL...

http://groups.google.com/group/comp.lang.c/browse_thread/thread/59680...

Basically the original poster wanted to remove the last character in
each array element. He never specified if he wanted to modify the
array in place. Anhow, it sort of looked like an interesting excercise
at the time. Anyhow, here is what I came up with...

We'll assume for simplicity that malloc() always returns a valid
pointer.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

void choparray(char **s, int nrows)
{
int i;
size_t j;

char **buf = malloc(nrows * sizeof(char *));
for(i = 0; i < nrows; i++) {
j = strlen(*s) -1;

You need to check that strlen(*s) is not 0.

buf = malloc(j * sizeof(char));

sizeof(char) is always 1.

strcpy(*(buf + i), *s);

Because j == strlen(*s) -1 you have 1 byte less than what you
actually need so this may overwrite memory that it shouldn't.

buf[j] = '\0';
*(s++); /*s[something]*/

s++ is sufficient.

}

for (i = 0; i < nrows; i ++) {
printf("%s\n", *(buf + i));
}

for(i = 0; i < nrows; i++)
free((void *)buf);
free((void *)buf);

Casting to void * serves no purpose and makes my eyes hurt.
Please don't do it.

}

int main(void)
{
int number;

char *stuff[]= {"test", "this", "input", "data"};
number = (sizeof stuff/sizeof stuff[0]);

choparray(stuff, number);

return 0;}

 

[Default User]

That fine, but what do you want it to do? Change it in place, or
return a new, modified, array? Right now you have something that's
neither. By the way, if you do modify it, you don't want to be
passing in those string literals. Undefined Behavior and all that.

Brian

I think changing it place would be pretty interesting. However, I
really have no idea how to go about doing it. The closest thing that I
can came up is modifying

char *stuff[]= {"test", "this", "input", "data"};

to something like

char stuff[][SOME_MADE_UP_NUMBER] = {"test", "this", "input", "data"};

Then then passing this to choparray(). And yes, I know that I forgot
to check malloc() for failure. Bad me.

Well, here's a program that does something along those lines. Not
terribly useful in my opinion, I can't really think of a real need case
for this, but you can look at it.


#include <stdio.h>
#include <string.h>

#define MAX_LEN 10

void choparray(char s[][MAX_LEN], size_t nstrs)
{
int i;

for (i = 0; i < nstrs; i++)
{
size_t len = strlen(s);
if (len > 0)
{
s[len-1] = '\0';
}
puts(s);
}
}

int main(void)
{
char stuff[][MAX_LEN] = {"test", "this", "input", "data"};

choparray(stuff, sizeof stuff/sizeof *stuff);

return 0;
}




Brian