Comparing string input to enum data type

[dtschoepe]

Greetings.

I am working on an assignment and can't seem to get the right concept
for something I'm attempting to do with enum data types.

I have defined the following in my code:

enum color {red, blue, green, yellow, black, purple, pink};

What I am doing now is reading from a text file a string (char *). I
wish to compare the value read into the array and determine which
color it is from the enum data type. How can I compare a string like
"red" to see if it exists in the values that are part of the
enumerated data type 'color'? I know I need to write some kind of
compare function, but I'm not getting anywhere figuring out how that
should work.

David

[Walter Roberson]

In article <11**********************@t69g2000cwt.googlegroups .com>,
dt*******@gmail.com <dt*******@gmail.comwrote:

>How can I compare a string like
"red" to see if it exists in the values that are part of the
enumerated data type 'color'?

You don't. By the time you run the program, the names assigned
to the enumerated values no longer have meaning; at that point,
C is working purely with their numeric values.

In your code, you can prepare a list of strings with the names
and compare against that.

There just might be a way to use a macro to name the enumeration
constant and to "stringify" and store away the name at the same
time, but if there is then it escapes me at the moment.
--
All is vanity. -- Ecclesiastes

[Bill Pursell]

On Feb 27, 8:45 pm, "[e-mail]" <[e-mail]:

Greetings.

I am working on an assignment and can't seem to get the right concept
for something I'm attempting to do with enum data types.

I have defined the following in my code:

enum color {red, blue, green, yellow, black, purple, pink};

What I am doing now is reading from a text file a string (char *). I
wish to compare the value read into the array and determine which
color it is from the enum data type. How can I compare a string like
"red" to see if it exists in the values that are part of the
enumerated data type 'color'? I know I need to write some kind of
compare function, but I'm not getting anywhere figuring out how that
should work.

If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...
--
Bill Pursell

[dtschoepe]

On Feb 27, 1:16 pm, "Bill Pursell" <[e-mail]:

>
If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

--
Bill Pursell

OK, so this brings up something else, the part where you have

sizeof "red"

Normally I have used it as sizeof(name) with the ()'s. Is this format
with ""'s used with enumerated data types? I am not familiar with this
way of using that operator.

What does this return, the int value of the enum type based on the
position in the list?

David

[Carl R. Davies]

<dt*******@gmail.comwrote in message
news:11**********************@p10g2000cwp.googlegr oups.com...

On Feb 27, 1:16 pm, "Bill Pursell" <[e-mail]:

>>
If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

--
Bill Pursell

OK, so this brings up something else, the part where you have

sizeof "red"

Normally I have used it as sizeof(name) with the ()'s. Is this format
with ""'s used with enumerated data types?

I believe sizeof only needs the () with types and not names or literals:

int x;

sizeof(int);
sizeof x;

[Yevgen Muntyan]

Bill Pursell wrote:

On Feb 27, 8:45 pm, "[e-mail]" <[e-mail]:

[snip]

>What I am doing now is reading from a text file a string (char *). I
wish to compare the value read into the array and determine which
color it is from the enum data type. How can I compare a string like
"red" to see if it exists in the values that are part of the
enumerated data type 'color'? I know I need to write some kind of
compare function, but I'm not getting anywhere figuring out how that
should work.

If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

Funny thing it actually works since 'sizeof "red"' is
greater than length of "red". Still, a rather strange
way to compare strings.

Yevgen

[Keith Thompson]

"Bill Pursell" <bi**********@gmail.comwrites:

On Feb 27, 8:45 pm, "[e-mail]" <[e-mail]:

>I am working on an assignment and can't seem to get the right concept
for something I'm attempting to do with enum data types.

I have defined the following in my code:

enum color {red, blue, green, yellow, black, purple, pink};

What I am doing now is reading from a text file a string (char *). I
wish to compare the value read into the array and determine which
color it is from the enum data type. How can I compare a string like
"red" to see if it exists in the values that are part of the
enumerated data type 'color'? I know I need to write some kind of
compare function, but I'm not getting anywhere figuring out how that
should work.

If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

Why not just strcmp("red", a)?

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

[Keith Thompson]

"dt*******@gmail.com" <dt*******@gmail.comwrites:
[...]

OK, so this brings up something else, the part where you have

sizeof "red"

Normally I have used it as sizeof(name) with the ()'s. Is this format
with ""'s used with enumerated data types? I am not familiar with this
way of using that operator.

There are two syntactic forms of the sizeof operator:

sizeof unary-expression

sizeof ( type-name )

A unary-expression can be a parenthesized expression, so given:

int obj;

either "sizeof obj" or "sizeof(obj)" is legal; in the latter, the
parentheses are part of the operand, not part of the syntax of the
sizeof operator itself.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

[Eric Sosman]

Bill Pursell wrote On 02/27/07 16:16,:

On Feb 27, 8:45 pm, "[e-mail]" <[e-mail]:

>>Greetings.

I am working on an assignment and can't seem to get the right concept
for something I'm attempting to do with enum data types.

I have defined the following in my code:

enum color {red, blue, green, yellow, black, purple, pink};

What I am doing now is reading from a text file a string (char *). I
wish to compare the value read into the array and determine which
color it is from the enum data type. How can I compare a string like
"red" to see if it exists in the values that are part of the
enumerated data type 'color'? I know I need to write some kind of
compare function, but I'm not getting anywhere figuring out how that
should work.

If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

Why not just strcmp("red", a) ?

--
Er*********@sun.com

[Racaille]

On Feb 27, 2:30 pm, "[e-mail]" <[e-mail]:

On Feb 27, 1:16 pm, "Bill Pursell" <[e-mail]:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;

OK, so this brings up something else, the part where you have

sizeof "red"

Normally I have used it as sizeof(name) with the ()'s. Is this format
with ""'s used with enumerated data types? I am not familiar with this
way of using that operator.

What does this return, the int value of the enum type based on the
position in the list?

no, it returns the 'size' of the literal, as if the literal was used
to initialize an array:

char a[] = "foobar";

sizeof a == sizeof(a) == sizeof "foobar" == sizeof("foobar") == 7

[user923005]

On Feb 27, 12:45 pm, "[e-mail]" <[e-mail]>
wrote:

Greetings.

I am working on an assignment and can't seem to get the right concept
for something I'm attempting to do with enum data types.

I have defined the following in my code:

enum color {red, blue, green, yellow, black, purple, pink};

What I am doing now is reading from a text file a string (char *). I
wish to compare the value read into the array and determine which
color it is from the enum data type. How can I compare a string like
"red" to see if it exists in the values that are part of the
enumerated data type 'color'? I know I need to write some kind of
compare function, but I'm not getting anywhere figuring out how that
should work.

>From the C-FAQ:

20.17: Is there a way to switch on strings?

A: Not directly. Sometimes, it's appropriate to use a separate
function to map strings to integer codes, and then switch on
those. Otherwise, of course, you can fall back on strcmp() and
a conventional if/else chain. See also questions 10.12, 20.18,
and 20.29.

References: K&R1 Sec. 3.4 p. 55; K&R2 Sec. 3.4 p. 58; ISO
Sec. 6.6.4.2; H&S Sec. 8.7 p. 248.

If you read the strings into an array called strclr, then you could
make a perfect hash with strclr[2] and switch on that.
red = d
blue = u
green = e
yellow = l
black = a
purple = r
pink = n

bcnu

[Ben Pfaff]

"user923005" <dc*****@connx.comwrites:

If you read the strings into an array called strclr, then you could
make a perfect hash with strclr[2] and switch on that.

I'd recommend not using strclr as the name, because it's in a
namespace reserved for the C standard library. It might
technically be OK to use it as the name of a local variable (I'm
not sure) but it's better to just avoid it.
--
Ben Pfaff
bl*@cs.stanford.edu
http://benpfaff.org

[CBFalconer]

Yevgen Muntyan wrote:

Bill Pursell wrote:

.... snip ...

>>
If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

Funny thing it actually works since 'sizeof "red"' is
greater than length of "red". Still, a rather strange
way to compare strings.

sizeof "red" had better be 4.

If it works you are comparing against a nul terminated string, and
you don't need the foofaraw of strncmp. Just use:

if (0 == strcmp("red", a)) c = red;
else if (....

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>

[websnarf]

On Feb 27, 12:45 pm, "[e-mail]" <[e-mail]>
wrote:

I am working on an assignment and can't seem to get the right concept
for something I'm attempting to do with enum data types.

I have defined the following in my code:

enum color {red, blue, green, yellow, black, purple, pink};

What I am doing now is reading from a text file a string (char *). I
wish to compare the value read into the array and determine which
color it is from the enum data type. How can I compare a string like
"red" to see if it exists in the values that are part of the
enumerated data type 'color'? I know I need to write some kind of
compare function, but I'm not getting anywhere figuring out how that
should work.

As others have posted, by the time your code is compiled these are
just values and not translated to strings. I've recently discovered
(don't know if I'm the first) a more maintainable way of dealing with
this sort of thing. If you make a seperate include file which just
looks like the following.

colors.h:
=========
invoke(red)
invoke(blue)
invoke(green)
....
invoke(pink)

--------------
Then in your .c file you would have something like:

#define invoke(c) c,
enum color {
#include "colors.h"
COLOR_ERROR
};
undef invoke

#define invoke(c) case c: return #c;
char * colorEnumToName (enum color c) {
switch (c) {
#include "colors.h"
default: break;
}
return NULL;
}
#undef invoke

#define invoke(c) if (0 == strcmp (#c, s)) return c;
enum color colorNameToEnum (const char * s) {
if (NULL == s) return COLOR_ERROR;
#include "colors.h"
return COLOR_ERROR;
}
#undef invoke

--------------
The idea being that you only need to manipulate the colors.h to add
more colors or change them in some way and you will maintain the
mappings from the enum to the string and vice versa with these
functions.

--
Paul Hsieh
http://www.pobox.com/~qed/
http://bstring.sf.net/

[dtschoepe]

On Feb 27, 1:16 pm, "Bill Pursell" <[e-mail]:

On Feb 27, 8:45 pm, "[e-mail]" <[e-mail]:

Greetings.

I am working on an assignment and can't seem to get the right concept
for something I'm attempting to do with enum data types.

I have defined the following in my code:

enum color {red, blue, green, yellow, black, purple, pink};

What I am doing now is reading from a text file a string (char *). I
wish to compare the value read into the array and determine which
color it is from the enum data type. How can I compare a string like
"red" to see if it exists in the values that are part of the
enumerated data type 'color'? I know I need to write some kind of
compare function, but I'm not getting anywhere figuring out how that
should work.

If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

--
Bill Pursell

Hmmmm, ok, I did some testing. I created the following program....

#include <stdio.h>

int main()
{

enum color { red, blue, brown, black, purple };

printf("red is %i\n", sizeof red);
}

This prints the number 4. My understanding of enum data types is that
the first one is 0, the second one is 1, etc. While this reply was
interesting and I learned something, it didn't really solve my
problem.

I hope one of the other replies caught my true question. By the way,
this resource is really good for people just starting out. I really
have enjoyed seeing the replies and reading through the other threads.

David

[Beej Jorgensen]

dt*******@gmail.com <dt*******@gmail.comwrote:

printf("red is %i\n", sizeof red);
This prints the number 4.

red is an integer type (like all enum expressions), and it's 4 bytes big
in your case. This doesn't have anything to do with the value of the
enum expression "red".

>My understanding of enum data types is that the first one is 0, the
second one is 1, etc.

Yes, unless the member is explicitly set to something else in the enum
specifier:

enum foo {
A, /* 0 */
B, /* 1 */
C, /* 2 */
D=12, /* 12 */
E, /* 13 */
F=2, /* 2 */
G /* 3 */
};

>While this reply was interesting and I learned something, it didn't
really solve my problem.

What people are getting at is that there's really no way to effortlessly
get the symbolic names of the enum members in your code. You have to
compare strings (or some clever variant) to accomplish your goal.

-Beej

[Yevgen Muntyan]

CBFalconer wrote:

Yevgen Muntyan wrote:

>Bill Pursell wrote:

... snip ...

>>If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

Funny thing it actually works since 'sizeof "red"' is
greater than length of "red". Still, a rather strange
way to compare strings.

sizeof "red" had better be 4.

What do you mean? It doesn't matter if it's 4 or 15, as long as it's
greater than 3 (and it is).

If it works you are comparing against a nul terminated string,

Of course, what else?

and
you don't need the foofaraw of strncmp.

Indeed. That's why I said it was rather strange way to compare strings.

Yevgen

[Nelu]

Yevgen Muntyan wrote:

CBFalconer wrote:

>Yevgen Muntyan wrote:

>>Bill Pursell wrote:

... snip ...

>>>If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...
Funny thing it actually works since 'sizeof "red"' is
greater than length of "red". Still, a rather strange
way to compare strings.

sizeof "red" had better be 4.

What do you mean? It doesn't matter if it's 4 or 15, as long as it's
greater than 3 (and it is).

sizeof "red" must be 4 no matter where you run the program. It
should give you the length of the array holding the string.

--
Ioan - Ciprian Tandau
tandau _at_ freeshell _dot_ org (hope it's not too late)
(... and that it still works...)

[Yevgen Muntyan]

Nelu wrote:

Yevgen Muntyan wrote:

>CBFalconer wrote:

>>Yevgen Muntyan wrote:
Bill Pursell wrote:

... snip ...
If you've red the string into a, and you want to set c:
>
if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...
Funny thing it actually works since 'sizeof "red"' is
greater than length of "red". Still, a rather strange
way to compare strings.
sizeof "red" had better be 4.

What do you mean? It doesn't matter if it's 4 or 15, as long as it's
greater than 3 (and it is).

sizeof "red" must be 4 no matter where you run the program. It
should give you the length of the array holding the string.

I say it 'sizeof "red" *is* 4', so I don't understand 'sizeof "red" had
better be 4'.

[Keith Thompson]

"dt*******@gmail.com" <dt*******@gmail.comwrites:

On Feb 27, 1:16 pm, "Bill Pursell" <[e-mail]:

[...]

>If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

Hmmmm, ok, I did some testing. I created the following program....

#include <stdio.h>

int main()
{

enum color { red, blue, brown, black, purple };

printf("red is %i\n", sizeof red);
}

[...]

The relevant expression in the code you quoted was
sizeof "red"
not
sizeof red

sizef "red" yields the size of the string literal, which is guaranteed
to be exactly 4. (sizeof red yields the same value as sizeof(int),
which might happen to be 4.)

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

[Flash Gordon]

dt*******@gmail.com wrote, On 28/02/07 04:50:

On Feb 27, 1:16 pm, "Bill Pursell" <[e-mail]:

>On Feb 27, 8:45 pm, "[e-mail]" <[e-mail]:

>>Greetings.
I am working on an assignment and can't seem to get the right concept
for something I'm attempting to do with enum data types.
I have defined the following in my code:
enum color {red, blue, green, yellow, black, purple, pink};
What I am doing now is reading from a text file a string (char *). I
wish to compare the value read into the array and determine which
color it is from the enum data type. How can I compare a string like
"red" to see if it exists in the values that are part of the
enumerated data type 'color'? I know I need to write some kind of
compare function, but I'm not getting anywhere figuring out how that
should work.

If you've red the string into a, and you want to set c:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

--
Bill Pursell

Hmmmm, ok, I did some testing. I created the following program....

#include <stdio.h>

int main()
{

enum color { red, blue, brown, black, purple };

printf("red is %i\n", sizeof red);

Bill did 'sizeof "red"' not 'sizeof red'. Note the quotation marks, they
are important. It means Bill was taking the size of the string literal
"red" where as you were taking the size of the enumeration constant red.
A bit like the difference between weighing an apple and weighing a piece
of paper with the word apple written on it.

}

This prints the number 4.

An unlucky coincidence. It could have been 2 (on a 16 bit system) or 8
(on a 64 bit system), or some other value, and you would get the same
result if you substituted purple for red.

My understanding of enum data types is that
the first one is 0, the second one is 1, etc.

Yes, but that has nothing to do with the size. I have a bag with 1000
dollars in it, how large is the bag? It is not 1000 dollars big it is
50cm by 50cm by 10cm.

While this reply was
interesting and I learned something, it didn't really solve my
problem.

Whatever you learnt I don't think it was either correct or what Bill was
trying to teach you.

I hope one of the other replies caught my true question.

Bills answer *did* catch your true question, read it again. Bill was
saying you have to read the input and then manually compare the input
against the strings "red", "purple" etc, and you cannot do this
automatically. Also, because of this, if you type the comparison string
as "reed" instead of "red" in your source code the compiler will not
notice because it is just a string and is not (as far as the compiler is
concerned) related to your enumeration.

As others have said, I would write Bill's code as
if (strcmp("red", a) == 0)
c = red;
else if ...

Note, I've change the spacing and swapped the comparison around as well
as changing to strcmp instead of strncmp. The important change is
changing to strcmp because it means less typing and less repetition of
information (hence less chance of getting it wrong), the other changes
are a matter of personal style, and obviously I prefer my style to
Bill's ;-)

By the way,
this resource is really good for people just starting out. I really
have enjoyed seeing the replies and reading through the other threads.

I'm glad you are enjoying it and, I hope, benefiting from it. Do not
worry if some of the threads go over your head.
--
Flash Gordon

[CBFalconer]

Yevgen Muntyan wrote:

Nelu wrote:

>Yevgen Muntyan wrote:

>>CBFalconer wrote:
Yevgen Muntyan wrote:
Bill Pursell wrote:
>
... snip ...
>If you've red the string into a, and you want to set c:
>>
>if( 0 == strncmp("red", a, sizeof "red") )
> c = red;
>else if ...
>
Funny thing it actually works since 'sizeof "red"' is
greater than length of "red". Still, a rather strange
way to compare strings.

sizeof "red" had better be 4.

What do you mean? It doesn't matter if it's 4 or 15, as long
as it's greater than 3 (and it is).

sizeof "red" must be 4 no matter where you run the program. It
should give you the length of the array holding the string.

I say it 'sizeof "red" *is* 4', so I don't understand 'sizeof
"red" had better be 4'.

It's an English idiom, meaning that if sizeof "red" isn't 4
something is seriously wrong.

I restored the blank separator lines your software mistakenly
removed.

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net>

[dtschoepe]

Here's what I ended up doing....

#define COLORS red, blue, green, yellow, black, purple, pink

..
..
..

enum color readColor(char * theFavColor)
{
enum colors { COLORS };
enum colors theColor;
if ( strcmp(color, "red") == 0 )
theColor = red;
else if ( strcmp(theFavColor, "blue") == 0)
theColor = blue;
else if ( strcmp(theFavColor, "green") == 0)
theColor = green;
else if ( strcmp(theFavColor, "yellow") == 0)
theColor = yellow;
else if ( strcmp(theFavColor, "black") == 0)
theColor = black;
else if ( strcmp(theFavColor, "purple") == 0)
theColor = purple;
else if ( strcmp(theFavColor, "pink") == 0)
theColor = pink;

return theColor;
}

This will work for now...

David

[Walter Roberson]

In article <11**********************@m58g2000cwm.googlegroups .com>,
dt*******@gmail.com <dt*******@gmail.comwrote:

>Here's what I ended up doing....

>#define COLORS red, blue, green, yellow, black, purple, pink

It isn't clear why you chose to #define those. Any change in them
needs to be reflected in the routine, so decoupling the symbols from
the routine does not buy you anything obvious.

>enum color readColor(char * theFavColor)
{
enum colors { COLORS };
enum colors theColor;
if ( strcmp(color, "red") == 0 )
theColor = red;
else if ( strcmp(theFavColor, "blue") == 0)
theColor = blue;
else if ( strcmp(theFavColor, "green") == 0)
theColor = green;
else if ( strcmp(theFavColor, "yellow") == 0)
theColor = yellow;
else if ( strcmp(theFavColor, "black") == 0)
theColor = black;
else if ( strcmp(theFavColor, "purple") == 0)
theColor = purple;
else if ( strcmp(theFavColor, "pink") == 0)
theColor = pink;

return theColor;
}

The variable named color is not set before you do the first
comparison (against "red"). The variable theColor is not set
if none of the choices match.
--
Is there any thing whereof it may be said, See, this is new? It hath
been already of old time, which was before us. -- Ecclesiastes

[Bill Pursell]

On Feb 27, 10:17 pm, Keith Thompson <[e-mail]:

"Bill Pursell" <[e-mail]:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

Why not just strcmp("red", a)?

Just a general tendency to refrain from using
strcmp. In this case, since one of the strings is
fixed, there's no security issue with strcmp (until
the code maintainer modifies the code...),
but I believe it's a good habit to use the strncmp
variant.

[Ben Pfaff]

"Bill Pursell" <bi**********@gmail.comwrites:

Just a general tendency to refrain from using
strcmp. In this case, since one of the strings is
fixed, there's no security issue with strcmp (until
the code maintainer modifies the code...),
but I believe it's a good habit to use the strncmp
variant.

What kind of a security issue would there be with strcmp?
--
"Some programming practices beg for errors;
this one is like calling an 800 number
and having errors delivered to your door."
--Steve McConnell

[Keith Thompson]

"Bill Pursell" <bi**********@gmail.comwrites:

On Feb 27, 10:17 pm, Keith Thompson <[e-mail]:

>"Bill Pursell" <[e-mail]:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

Why not just strcmp("red", a)?

Just a general tendency to refrain from using
strcmp. In this case, since one of the strings is
fixed, there's no security issue with strcmp (until
the code maintainer modifies the code...),
but I believe it's a good habit to use the strncmp
variant.

strcmp() is a security problem only if one of the arguments isn't a
null-terminated string. I'd be more afraid of getting the strncmp()
arguments wrong than of passing bad values to strcmp().

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

[dtschoepe]

On Feb 28, 10:21 am, [e-mail] (Walter Roberson)
wrote:

#define COLORS red, blue, green, yellow, black, purple, pink

It isn't clear why you chose to #define those. Any change in them
needs to be reflected in the routine, so decoupling the symbols from
the routine does not buy you anything obvious.

enum color readColor(char * theFavColor)
{
enum colors { COLORS };
enum colors theColor;
if ( strcmp(color, "red") == 0 )
theColor = red;
else if ( strcmp(theFavColor, "blue") == 0)
theColor = blue;
else if ( strcmp(theFavColor, "green") == 0)
theColor = green;
else if ( strcmp(theFavColor, "yellow") == 0)
theColor = yellow;
else if ( strcmp(theFavColor, "black") == 0)
theColor = black;
else if ( strcmp(theFavColor, "purple") == 0)
theColor = purple;
else if ( strcmp(theFavColor, "pink") == 0)
theColor = pink;

return theColor;
}

The variable named color is not set before you do the first
comparison (against "red"). The variable theColor is not set
if none of the choices match.

I goofed there on the 'color' reference, it should also be
'theColor'.

I'll add a default assignment at the bottom, not a bad idea.

This is just a small assignment where we are creating a structure that
represents a person and one of the characteristics is a favorite
color. I then build a linked list of Persons and perform various
operations on that list.

I used #define so I would have less places to edit in case I added a
color, since I use it in at least 2 places.

David

[Keith Thompson]

"dt*******@gmail.com" <dt*******@gmail.comwrites:

On Feb 28, 10:21 am, [e-mail] (Walter Roberson)
wrote:

>#define COLORS red, blue, green, yellow, black, purple, pink

It isn't clear why you chose to #define those. Any change in them
needs to be reflected in the routine, so decoupling the symbols from
the routine does not buy you anything obvious.

[snip]

I used #define so I would have less places to edit in case I added a
color, since I use it in at least 2 places.

You only use the COLORS macro in one place in the code you showed us,
and I can't think of a good reason to use it more than once. You do,
however, have string literals "red", "blue", etc., which would have to
be individually updated if you changed the type.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

[Yevgen Muntyan]

Keith Thompson wrote:

"Bill Pursell" <bi**********@gmail.comwrites:

>On Feb 27, 10:17 pm, Keith Thompson <[e-mail]:

>>"Bill Pursell" <[e-mail]:
if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...
Why not just strcmp("red", a)?

Just a general tendency to refrain from using
strcmp. In this case, since one of the strings is
fixed, there's no security issue with strcmp (until
the code maintainer modifies the code...),
but I believe it's a good habit to use the strncmp
variant.

strcmp() is a security problem only if one of the arguments isn't a
null-terminated string. I'd be more afraid of getting the strncmp()
arguments wrong than of passing bad values to strcmp().

Absolutely. I have actually seen code like

if (strncmp (foo, "blah", 4) == 0)

which was *meant* to compare foo to "blah" (i.e. do what strcmp()
does). And the form used here is totally bad, since it breaks
if string literal is replaced with a pointer or an array which
isn't exactly "red".

strncmp ("red", foo, strlen ("red") + 1)

would be safer, but then if foo is not zero-terminated, it could
as well go outside foo, so the safer would be

strncmp ("red", foo, MAX (strlen ("red") + 1, max_len_of_foo))

which is totally ridiculous.

Yevgen

[Racaille]

On Feb 28, 12:32 pm, Keith Thompson <[e-mail]:

"Bill Pursell" <[e-mail]:

On Feb 27, 10:17 pm, Keith Thompson <[e-mail]:

"Bill Pursell" <[e-mail]:

if( 0 == strncmp("red", a, sizeof "red") )
c = red;
else if ...

Why not just strcmp("red", a)?

Just a general tendency to refrain from using
strcmp. In this case, since one of the strings is
fixed, there's no security issue with strcmp (until
the code maintainer modifies the code...),
but I believe it's a good habit to use the strncmp
variant.

strcmp() is a security problem only if one of the arguments isn't a
null-terminated string. I'd be more afraid of getting the strncmp()
arguments wrong than of passing bad values to strcmp().

both strcmp and strncmp may be a problem if either of their arguments
is not zero-terminated.

for instance, on a system with virtual memory, one of arguments may
point
just below the end of a page, and trying to read beyond the end of
the page may crash the application (or some device may have the
registers
wired there, etc).

[Racaille]

On Feb 28, 4:18 pm, "Racaille" <[e-mail]:

On Feb 28, 12:32 pm, Keith Thompson <[e-mail]:

strcmp() is a security problem only if one of the arguments isn't a
null-terminated string. I'd be more afraid of getting the strncmp()
arguments wrong than of passing bad values to strcmp().

both strcmp and strncmp may be a problem if either of their arguments
is not zero-terminated.

I misread your post, sorry :)

[Richard Heathfield]

Racaille said:

<snip>

both strcmp and strncmp may be a problem if either of their arguments
is not zero-terminated.

*ANY* function may be a problem if the programmer doesn't use it
properly. There is no inherent danger whatsoever in using strcmp if you
follow the interface specification correctly. The same cannot be said
of *all* standard C functions, but it can certainly be said of strcmp.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at the above domain, - www.

[Martin Golding]

On Wed, 28 Feb 2007 10:17:40 -0800, dt*******@gmail.com wrote:

Here's what I ended up doing....

#define COLORS red, blue, green, yellow, black, purple, pink

enum color readColor(char * theFavColor) {
enum colors { COLORS };
enum colors theColor;
if ( strcmp(color, "red") == 0 )
theColor = red;
else if ( strcmp(theFavColor, "blue") == 0)
theColor = blue;
else if ( strcmp(theFavColor, "green") == 0)
theColor = green;
else if ( strcmp(theFavColor, "yellow") == 0)
theColor = yellow;
else if ( strcmp(theFavColor, "black") == 0)
theColor = black;
else if ( strcmp(theFavColor, "purple") == 0)
theColor = purple;
else if ( strcmp(theFavColor, "pink") == 0)
theColor = pink;

return theColor;
}
This will work for now...

If returning an uninitialized value for non-matching color works.
(It does if you can count on your data being valid or are willing
to be abusive if it isn't.)

I prefer to separate data from code, and like abusing macros.

Here's a portable solution, which collects the data in one place
(suitable for a header file) but pays the penalty of a loop versus
your compares. This and your solution allow frequent colors to be
ordered first for efficiency:
#include <stdio.h>
#include <string.h>

#define COLOR(color) {color, #color}

struct colordef {
int value;
char *name;
};

enum colors {none = 0, red, blue, green, yellow, black, purple, pink};
struct colordef colortbl[] = {
COLOR(red),
COLOR(blue),
COLOR(green),
COLOR(yellow),
COLOR(black),
COLOR(purple),
COLOR(pink),
{0,0}
};
enum colors
readColor(char *theFavColor)
{
int i;
enum colors theColor;

i = 0;
while (colortbl[i].value) {
if (!strcmp(theFavColor, colortbl[i].name)) break;
i++;
}

return colortbl[i].value;
}
int main(void)
{
printf("Red is %d\n", readColor("red"));
printf("Pink is %d\n", readColor("pink"));
printf("Puce is %d\n", readColor("puce"));

return 0;
}

[1037] make color1
cc -O1 -DDEBUG -g -Wmissing-prototypes -Wstrict-prototypes -Wimplicit color1.c -o color1
color1.c:27: warning: no previous prototype for 'readColor'
[1038] ./color1
Red is 1
Pink is 7
Puce is 0

Another alternative, which associates the color name to its subscript.
'none' (no match) here is zero, if it were allowed to float the loop
would not have to be reversed. I believe the initialization is C99,
somebody will huff if it's a gcc extension:

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

#define COLOR(color) [color] = #color

enum colors {none = 0, red, blue, green, yellow, black, purple, pink};
/* If the maintainers promise to be good, this could be simplified
to {NULL, "red", "blue", ... */
char *colortbl[] = {
NULL,
COLOR(red),
COLOR(blue),
COLOR(green),
COLOR(yellow),
COLOR(black),
COLOR(purple),
COLOR(pink),
};
#define NUMCOLORS ((sizeof colortbl/sizeof colortbl[0]))

enum colors
readColor(char *theFavColor)
{
int i;
enum colors theColor;

for (i = NUMCOLORS-1; i 0; i--)
if (!strcmp(colortbl[i], theFavColor)) break;

return i;
}
int main(void)
{
printf("Red is %d\n", readColor("red"));
printf("Pink is %d\n", readColor("pink"));
printf("Puce is %d\n", readColor("puce"));

return 0;
}

[1035] make color2
cc -O1 -DDEBUG -g -Wmissing-prototypes -Wstrict-prototypes -Wimplicit color2.c -o color2
color2.c:23: warning: no previous prototype for 'readColor'
[1036] ./color2
Red is 1
Pink is 7
Puce is 0

Martin
--
Martin Golding | You don't need computers to screw things up,
DoD #236 BMWMOA #55952 | but it does make the work go faster.

[Richard Bos]

Yevgen Muntyan <mu****************@tamu.eduwrote:

strncmp ("red", foo, strlen ("red") + 1)

would be safer, but then if foo is not zero-terminated, it could
as well go outside foo, so the safer would be

strncmp ("red", foo, MAX (strlen ("red") + 1, max_len_of_foo))

I think you meant MIN instead of MAX.

which is totally ridiculous.

This, however, is as true as a very true thing.

Richard

[Yevgen Muntyan]

Richard Bos wrote:

Yevgen Muntyan <mu****************@tamu.eduwrote:

>strncmp ("red", foo, strlen ("red") + 1)

would be safer, but then if foo is not zero-terminated, it could
as well go outside foo, so the safer would be

strncmp ("red", foo, MAX (strlen ("red") + 1, max_len_of_foo))

I think you meant MIN instead of MAX.

That's why one should worry about correctness of strncmp() arguments :)

Yevgen