weird problem with strcmp()

[rabbits77]

x-no-archive: yes

I am playing around with some code trying to make a toy
command shell.
What is below is very much a work in progress but what I am finding
is that the strcmp()s are not behaving as I expected.
That is if I enter a command line like
run something
the command "run" is correctly parsed by strtok
but the
if (!strcmp(command, "run"))
does not print the line saying it was recognized.
Why is that?










#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <strings.h>

#define MAXSIZE 4096
#define ONEBYTE 1
#define NUMBYTES(x) x*2

void execute_command();

int main(){
int n;
int num_chars=-1;
char char_buf[1];
char* command_line;
command_line=(char*)malloc(MAXSIZE);
bzero(command_line, MAXSIZE);
fcntl(STDIN_FILENO, F_SETFL, O_ASYNC); // set to
asynchronous I/O
while((n=read(STDIN_FILENO, char_buf,
ONEBYTE))==ONEBYTE){//read one byte at a time
num_chars++;
if(char_buf[0]=='\n'){
/*write(STDOUT_FILENO, command_line, NUMBYTES(num_chars));*/
execute_command(command_line);
bzero(command_line, MAXSIZE);
num_chars=-1;
}
else{
strcat(command_line,char_buf);
}
}
}

void execute_command(char* command_line){
char* command = strtok(command_line, " ");
char* argument = strtok(NULL,"");
printf("command: \"%s\"\n",command);
printf("argument: \"%s\"\n",argument);
command[3]='\0';
if (!strcmp(command, "run")) {
printf("run: \"%s\"\n",command);
}
if (strcmp(command, "show")) {
printf("show: \"%s\"\n",command);
}
if (strcmp(command, "kill")) {
printf("kill: \"%s\"\n",command);
}
}

 

[Seebs]

x-no-archive: yes

Why do you do this? It's annoying.

I am playing around with some code trying to make a toy
command shell.
What is below is very much a work in progress but what I am finding
is that the strcmp()s are not behaving as I expected.
That is if I enter a command line like
run something
the command "run" is correctly parsed by strtok
but the
if (!strcmp(command, "run"))
does not print the line saying it was recognized.
Why is that?

#include <fcntl.h>

It'd be really helpful if you'd strip your program to something
that doesn't depend on system-specific stuff, to make it easier for
people who aren't on the same system you are to look at your
code.

#define ONEBYTE 1

This is really annoying. Don't do that. "1" is not
a magic number, and "ONEBYTE" implies that you might think
it's something other than 1.

char char_buf[1];

bzero(command_line, MAXSIZE);

Don't use bzero, it's not portable.

strcat(command_line,char_buf);

This is undefined behavior. strcat needs its argument to be an
actual string, meaning, it must be null-terminated. Since the array
is only 1 character long, it can't be. Also, this is insanely
inefficient -- even if it worked, you'd be using O(N^2) time for a
command line of length N.

void execute_command(char* command_line){
char* command = strtok(command_line, " ");
char* argument = strtok(NULL,"");
printf("command: \"%s\"\n",command);
printf("argument: \"%s\"\n",argument);
command[3]='\0';

This probably shouldn't be here.

if (!strcmp(command, "run")) {
printf("run: \"%s\"\n",command);
}

if (strcmp(command, "show")) {
printf("show: \"%s\"\n",command);
}
if (strcmp(command, "kill")) {
printf("kill: \"%s\"\n",command);
}
}

Okay, let's try this program:

int
main(void)
{
char cmd[] = "run foo";
execute_command(cmd);
return 0;
}

with that. We get:

command: "run"
argument: "foo"
run: "run"
show: "run"
kill: "run"

Which is exactly what we'd expect (note that you have the sense of the
later strcmp() reversed).

So. Strip the program down to something comparable, see whether it works.
If it does, the problem is in the vaguely-unix-like code; my first guess
would be that your string contains non-printing characters which aren't
visible to you in the printf but are affecting the return value of
strcmp, and that fixing the way you populate that string will solve your
problem.

-s

 

[rabbits77]

x-no-archive: yes

Oh, and please ignore the line
command[3]='\0'
that was accidentally added to what I posted
and is not part of the problem.
Thanks!

 

[spinoza1111]

x-no-archive: yes

I am playing around with some code trying to make a toy
command shell.
What is below is very much a  work in progress but what I am finding
is that the strcmp()s are not behaving as I expected.
That is if I enter a command line like
run something
the command "run" is correctly parsed by strtok
but the
if (!strcmp(command, "run"))
does not print the line saying it was recognized.
Why is that?

#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <strings.h>

#define MAXSIZE 4096
#define ONEBYTE 1
#define NUMBYTES(x) x*2

void execute_command();

int main(){
        int n;
        int num_chars=-1;
        char char_buf[1];
        char* command_line;
         command_line=(char*)malloc(MAXSIZE);
         bzero(command_line, MAXSIZE);
         fcntl(STDIN_FILENO, F_SETFL, O_ASYNC);     // set to
asynchronous I/O
         while((n=read(STDIN_FILENO, char_buf,
ONEBYTE))==ONEBYTE){//read one byte at a time
            num_chars++;
            if(char_buf[0]=='\n'){
                    /*write(STDOUT_FILENO, command_line, NUMBYTES(num_chars));*/
                    execute_command(command_line);
                bzero(command_line, MAXSIZE);
                    num_chars=-1;
            }
            else{
                    strcat(command_line,char_buf);
            }
        }

}

void execute_command(char* command_line){
        char* command = strtok(command_line, " ");
        char* argument = strtok(NULL,"");
        printf("command: \"%s\"\n",command);
        printf("argument: \"%s\"\n",argument);
        command[3]='\0';
        if (!strcmp(command, "run")) {
         printf("run: \"%s\"\n",command);
        }
        if (strcmp(command, "show")) {
         printf("show: \"%s\"\n",command);
        }
        if (strcmp(command, "kill")) {
         printf("kill: \"%s\"\n",command);
        }



}

1. You negate the strcmp to "run" which means of course when it is
present it won't be printed.

2. You set command[3] to zero. Assuming you even need to do this, it
will work for run and fail for show and kill.

 

[Barry Schwarz]

x-no-archive: yes

I am playing around with some code trying to make a toy
command shell.
What is below is very much a work in progress but what I am finding
is that the strcmp()s are not behaving as I expected.
That is if I enter a command line like
run something
the command "run" is correctly parsed by strtok
but the
if (!strcmp(command, "run"))
does not print the line saying it was recognized.
Why is that?










#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <strings.h>

#define MAXSIZE 4096
#define ONEBYTE 1
#define NUMBYTES(x) x*2

void execute_command();

int main(){
int n;
int num_chars=-1;
char char_buf[1];
char* command_line;
command_line=(char*)malloc(MAXSIZE);

You don't need the cast in C.

bzero(command_line, MAXSIZE);

Is there some reason you prefer the system specific bzero to the
standard library function memset?

fcntl(STDIN_FILENO, F_SETFL, O_ASYNC); // set to
asynchronous I/O
Why?

while((n=read(STDIN_FILENO, char_buf,
ONEBYTE))==ONEBYTE){//read one byte at a time

Is there some reason you prefer the system specific read to the
standard library function fread or getchar or fgetc.

num_chars++;
if(char_buf[0]=='\n'){
/*write(STDOUT_FILENO, command_line, NUMBYTES(num_chars));*/
execute_command(command_line);
bzero(command_line, MAXSIZE);
num_chars=-1;
}
else{
strcat(command_line,char_buf);

strcat requires both operands to point to strings. If bzero sets
command_line[0] to '\0', then that will satisfy the first operand. But
char_buf does not point to (nor contain) a string. It is only one
char long and that character is what was read from stdin. There is no
trailing '\0' to terminate the string. You have invoked undefined
behavior and you have no idea what the final contents of the memory
pointed to by command_line is.

Since you have already kept count of the number of character, a simple
command_line[num_chars-1] = char_buf[0];
would eliminate this problem.

}
}
}

void execute_command(char* command_line){
char* command = strtok(command_line, " ");
char* argument = strtok(NULL,"");
printf("command: \"%s\"\n",command);
printf("argument: \"%s\"\n",argument);
command[3]='\0';

This would cause a command of "runaway" to be treated as if it were
"run". Is that your intent? Or didn't you realize that the first
call to strtok would terminate the first "token" so your strcmp would
work without this?

if (!strcmp(command, "run")) {
printf("run: \"%s\"\n",command);
}
if (strcmp(command, "show")) {

Since you manually set command[3] to '\0', this can never be true.

 

[rabbits77]

x-no-archive: yes
Seebs wrote:
[snip]

So. Strip the program down to something comparable, see whether it works.
If it does, the problem is in the vaguely-unix-like code; my first guess
would be that your string contains non-printing characters which aren't
visible to you in the printf but are affecting the return value of
strcmp, and that fixing the way you populate that string will solve your
problem.

Thanks for the advice.
The problem I was seeing came exclusively from
the fact that I was trying to read in a single character
at a time(remember this is just toy code...I know this
is completely inefficient). I was restricting myself to
using read() which takes a char* as an arg, not a single
char. So, in order to remain faithful to my restriction
I used a small char array to read in a single character
at a time. I did not correctly add a null termination
character '\0' to this array which caused garbage
characters to come into play when I then proceeded to
use strcat.
Anyway, thanks for the help!

---------------------
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <strings.h>

#define MAXSIZE 4096
#define ONEBYTE 1
#define NUMBYTES(x) x*2

int execute_command();

int main(){
int n;
int num_chars=-1;
char char_buf[2];
char* command_line;
command_line=(char*)malloc(MAXSIZE);
bzero(command_line, MAXSIZE);
while((n=read(STDIN_FILENO, char_buf,
ONEBYTE))==ONEBYTE){//read one byte at a time
char_buf[1]='\0';
num_chars++;
if(char_buf[0]=='\n'){
execute_command(command_line);
bzero(command_line, MAXSIZE);
num_chars=-1;
}
else{
strcat(command_line,char_buf);
}
}
}

int execute_command(char* command_line){
char* command = strtok(command_line, " ");
char* argument = strtok(NULL,"");
if (!strcmp(command, "run")) {
printf("run: \"%s\"\n",command);
}
if (!strcmp(command, "show")) {
printf("show: \"%s\"\n",command);
}
if (!strcmp(command, "kill")) {
printf("kill: \"%s\"\n",command);
}
}

---------------------
$ ./a.out
run puke
run: "run"
show puke
show: "show"
kill puke
kill: "kill"
^C

 

[Seebs]

x-no-archive: yes

Again, why are you doing this?

The problem I was seeing came exclusively from
the fact that I was trying to read in a single character
at a time(remember this is just toy code...I know this
is completely inefficient). I was restricting myself to
using read() which takes a char* as an arg, not a single
char. So, in order to remain faithful to my restriction
I used a small char array to read in a single character
at a time. I did not correctly add a null termination
character '\0' to this array which caused garbage
characters to come into play when I then proceeded to
use strcat.
Ayup.

#include <strings.h>

Don't do this, either. It's <string.h>.

You'd benefit a lot from trying to learn standard C rather than picking
up weird and obsolete habits that probably made sense on some Unix system
back in the early 80s.

-s

 

[Ersek, Laszlo]

Don't do this, either. It's <string.h>.

You'd benefit a lot from trying to learn standard C rather than picking
up weird and obsolete habits that probably made sense on some Unix system
back in the early 80s.

Not to dispute this, but <strings.h> is not legacy when talking about
UNIX(R). bzero() is.

SUSv1: C435 page 812 (physical page 840)
SUSv2: http://www.opengroup.org/onlinepubs/007908775/xsh/strings.h.html
SUSv3: http://www.opengroup.org/onlinepubs/000095399/basedefs/strings.h.html
SUSv4: http://www.opengroup.org/onlinepubs/9699919799/basedefs/strings.h.html

Cheers,
lacos

 

[Noob]

Why do you do this? It's annoying.

"this" is ambiguous. You might be asking

1) why is X-No-Archive set
2) why is X-No-Archive set in the body of the message

http://en.wikipedia.org/wiki/X-No-Archive

To rabbits77:

"Mozilla Thunderbird has the ability to insert custom fields
into the header of both email and Usenet messages."

http://kb.mozillazine.org/Custom_headers

 

[Keith Thompson]

Why do you do this? It's annoying.

That's odd. I don't see an "x-no-archive" in the original message.

 

[Seebs]

Not to dispute this, but <strings.h> is not legacy when talking about
UNIX(R).

I would say that it is. Everyone is guaranteed to have <string.h>, and
<strings.h> exists only for historical compatibility. I wouldn't recommend
that people use an interface which is only there for backwards
compatibility when there's a better-supported modern one. (Among other
things, it's one more thing to fix when porting to another system.)

-s

 

[Seebs]

That's odd. I don't see an "x-no-archive" in the original message.

It's in the body of the post. But some interfaces may helpfully
hide that.

-s

 

[Seebs]

What about strcasecmp? It is in strings.h? Posix/Bsd compliant?

On the systems I've used that function on, it was also accessible from
<string.h>.

But I wouldn't usually use it, just because of the portability issues.

-s

 

[Richard Bos]

That's odd. I don't see an "x-no-archive" in the original message.

Then your eMacs is broken (so what else is new), because it definitely
is there.

Richard

 

[Kenny McCormack]

That's odd. I don't see an "x-no-archive" in the original message.

Another episode of... CLC regs eating their own!

--
(This discussion group is about C, ...)

Wrong. It is only OCCASIONALLY a discussion group
about C; mostly, like most "discussion" groups, it is
off-topic Rorsharch revelations of the childhood
traumas of the participants...

 

[Keith Thompson]

It's in the body of the post. But some interfaces may helpfully
hide that.

Ah, you're right. Yes, my newsreader hid it from me; I had to save
the article to a file to be able to see it.

(For what it's worth, I agree that using the x-no-archive header
without a good reason is annoying.)

 

[Keith Thompson]

Then your eMacs is broken (so what else is new), because it definitely
is there.

It's a feature of Gnus, not of Emacs, and like most things it's
configurable. I won't argue whether it's "broken" or not, but I've
just reconfigured it.

 

[TheGist]

On the systems I've used that function on, it was also accessible from
<string.h>.

But I wouldn't usually use it, just because of the portability issues.

The OP was just playing around with some toy problem.
Why bother harping about "portability"?
What does that even mean in this case?
Surely bzero is available on windows(cygwin), mac os x,
and any modern *nix variant. What platform do you
suspect the OP would be porting his exercise to? LOL.

 

[Seebs]

The OP was just playing around with some toy problem.
Yup.

Why bother harping about "portability"?

Because sloppy work is habit forming.

What does that even mean in this case?

It means that if you get in the habit of writing code which relies on
non-portable things without good justifications, you will spend more
time dealing with the aftermath later, usually at the least convenient
possible time.

Surely bzero is available on windows(cygwin), mac os x,
and any modern *nix variant.

We were talking about strcasecmp(), which is not as widely available
as you might think -- largely because, while the functionality is common,
some systems call it stricmp().

What platform do you
suspect the OP would be porting his exercise to?

I have no idea. I don't necessarily expect the OP to use the same program
again, ever. However, I do expect the OP to write code which reflects
the habits and assumptions that have been developed through years of writing
little test programs and exercises, and it turns out to be very handy if
those habits and assumptions lead to, in general, more portable code.

-s

 

[Keith Thompson]

The OP was just playing around with some toy problem.
Why bother harping about "portability"?

It's never too early to develop good habits.

What does that even mean in this case?
Surely bzero is available on windows(cygwin), mac os x,
and any modern *nix variant. What platform do you
suspect the OP would be porting his exercise to? LOL.

Here's an excerpt from the man page for bzero on one system:

CONFORMING TO
4.3BSD. This function is deprecated (marked as LEGACY in
POSIX.1-2001): use memset(3) in new programs. POSIX.1-2008
removes the specification of bzero().

memset() is guaranteed to exist in any conforming hosted C
implementation, and it almost certainly always will be as long as
there's a language named "C". bzero() is not guaranteed to exist,
and as time passes there will probably be more and more systems
where either it doesn't exist, or you have to take some special
action to enable it.

(If you're using a *really* old implementation, it's conceivable that
bzero() might be available and memset() might not be. But if you're
using such a system, you should already be aware of these issues.)