Re: weird problem with strcmp()
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
--
Copyright 2010, all wrongs reversed. Peter Seebach / usenet...@seebs.net
http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
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 <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';
> 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
wrote:
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.
> printf("show: \"%s\"\n",command);
> }
> if (strcmp(command, "kill")) {
Ditto.
> printf("kill: \"%s\"\n",command);
> }
>}
--
Remove del for email
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.
Ersek, Laszlo
> On 2010-04-02, rabbits77 <rabb...@my-deja.com> wrote:
>> #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.
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
> rabbits77 wrote:
>
>> x-no-archive: yes
>
> 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."
Keith Thompson
> On 2010-04-01, rabbits77 <rabb...@my-deja.com> wrote:
>> x-no-archive: yes
>
> Why do you do this? It's annoying.
That's odd. I don't see an "x-no-archive" in the original message.
--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
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.)
Seebs
> Seebs <usenet...@seebs.net> writes:
>> On 2010-04-01, rabbits77 <rabb...@my-deja.com> wrote:
>>> x-no-archive: yes
>> Why do you do this? It's annoying.
> 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.
Branimir Maksimovic
Seebs <usenet...@seebs.net> wrote:
> On 2010-04-02, Ersek, Laszlo <la...@caesar.elte.hu> wrote:
> > 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
What about strcasecmp? It is in strings.h? Posix/Bsd compliant?
Greets!
Sometimes online sometimes not
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.
Richard Bos
> Seebs <usenet...@seebs.net> writes:
> > On 2010-04-01, rabbits77 <rabb...@my-deja.com> wrote:
> >> x-no-archive: yes
> >
> > Why do you do this? It's annoying.
>
> 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
Keith Thompson <ks...@mib.org> wrote:
>Seebs <usenet...@seebs.net> writes:
>> On 2010-04-01, rabbits77 <rabb...@my-deja.com> wrote:
>>> x-no-archive: yes
>>
>> Why do you do this? It's annoying.
>
>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
> On 2010-04-02, Keith Thompson <ks...@mib.org> wrote:
>> Seebs <usenet...@seebs.net> writes:
>>> On 2010-04-01, rabbits77 <rabb...@my-deja.com> wrote:
>>>> x-no-archive: yes
>
>>> Why do you do this? It's annoying.
>
>> 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.
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
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 2010-04-02, Branimir Maksimovic <bm...@hotmail.com> wrote:
>> 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.
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
> Seebs wrote:
>> 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.
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.
Keith Thompson
> Seebs wrote:
>> On 2010-04-02, Branimir Maksimovic <bm...@hotmail.com> wrote:
>>> 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.
> 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.)
Nick
> On 2010-04-03, TheGist <the...@nospam.net> wrote:
>> Seebs wrote:
>>> 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.
>
> 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().
Which is why my messy-bits-of-stuff file has caseless_strcmp #defined to
one or the other. If you don't have either you can write your own, of
course. I also keep in the right part of the namespace, and have a more
meaningful name as well (the one thing that strcasecmp isn't is "case").
I actually use autoconf to find out which is around, but just doing it
by hand is a big step forwards.
It took me years to realise the benefits of these sorts of things.
--
Online waterways route planner | http://canalplan.eu
Plan trips, see photos, check facilities | http://canalplan.org.uk