K&R 2 exercise 2-3

[Herrcho]

Hi~ i've studied C for a few months myself,

and i'd appreciate it if anyone could improve my coding or correct it.

the following is my solution to the K&R exercise 2-3

"Write the function htoi(s), which converts a string of hexademical digits
(including an optional 0x or 0X) into its equivalent integer value.
The allowable digits are 0 through 9, a through f, and A throught F."
//*************** *************** *************** *************** **************

#include <stdio.h>

int isxdigit2(int c)
{
if ( (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') || (c >= '0' && c <= '9') )
return 1;
else
return 0;
}

int tolower2(int c)
{
if (c >= 'A' && c <= 'Z')
return c+32;
else
return c;
}

int power2(int base, int num)
{
int sum;
for (sum = 1; num >0 ; num --)
sum *= base;
return sum;
}

int char_to_num(int c)
{
if (c >= '0' && c <= '9')
{
return c - 48;
}
else
{
return 10 + (tolower2(c) - 'a');
}
}

int htoi(char *c)
{
int i, k, prefix = 0;
size_t sum = 0;

if (c[0] == '0' && tolower2(c[1]) == 'x')
prefix = 1;

for (i = (prefix == 1)? 2:0 ; c[i] ;i++ )
{
if (!isxdigit2(c[i]) )
{
printf("Wrong hexa number\n");
return 0;
}
c[i] = char_to_num(c[i]);
}

for (k = (prefix == 1)? 2 : 0 ; k <= i-1 ; ++k )
{
sum += c[k] * power2(16, i-1-k);
}

return sum;
}

int main()
{
char c[] = "0xAB";
printf("%u", htoi(c));

return 0;
}

//*************** *************** *************** *************** ******

when i change char c[] to char *c in main(),
it shows error, why ??

Thanks..

[Chris Torek]

In article <news:40******* ********@yahoo. com>
CBFalconer <cb********@wor ldnet.att.net> writes:

Why make any assumptions?

/* also useful for reverse conversions */
static char[] hexchars = "0123456789abcd efABCDEF";
You need to test these things before posting -- that should
be "static char hexchars[]". :-)

More seriously, it might be worth making it a readonly array,
of type "const char".

[snippage] if (h > 15) h = -1; /* Exercise - why this */

If you want to get rid of this, there is always memchr().
--
In-Real-Life: Chris Torek, Wind River Systems
Salt Lake City, UT, USA (40°39.22'N, 111°50.29'W) [phone]
email: forget about it http://web.torek.net/torek/index.html
Reading email is like searching for food in the garbage, thanks to spammers.

[pete]

Chris Torek wrote:

In article <news:40******* ********@yahoo. com>
CBFalconer <cb********@wor ldnet.att.net> writes:

Why make any assumptions?

/* also useful for reverse conversions */
static char[] hexchars = "0123456789abcd efABCDEF";

You need to test these things before posting -- that should
be "static char hexchars[]". :-)

More seriously, it might be worth making it a readonly array,
of type "const char".

[snippage]

if (h > 15) h = -1; /* Exercise - why this */

If you want to get rid of this, there is always memchr().

or

s = (h != '\0' ? strchr(hexchars , h) : NULL);

--
pete

[Joe Wright]

CBFalconer wrote:

Joe Wright wrote:

... snip ...

/*
We will examine a printing hex character and determine its bin value.
Of course the numerics will translate directly. The alphas are a
special case as both 'a' and 'A' will evaluate to 10.
Input must be int within the ranges '0'..'9', 'A'..'F' and 'a'..'f'.

Some assumptions:

That '0',,'9', 'A'..'F' and 'a'..'f' are contiguous in the set.
'0'..'9' is guaranteed but the others are not. Oh well, they are
contiguous in ASCII and EBCDIC.
*/

/* Returns a value 0..15 for hex digits or -1 on failure. */
int h2b(int h) {

Why make any assumptions?

/* also useful for reverse conversions */
static char[] hexchars = "0123456789abcd efABCDEF";

/* Returns a value 0..15 for hex digits or -1 on failure. */
int h2b(int h)
{
char * s;

if (NULL == (s = strchr(hexchars , h))) h = -1;
else {
h = s - hexchars;
if (h > 15) h = h - 6;
}
if (h > 15) h = -1; /* Exercise - why this */
return h;
}

The string should be ..

static char hexchars[] = "0123456789abcd efABCDEF";

Exercise? There is a NUL at hexchars[22]. If h started off 0 strchr()
will find the NUL and 'h = s - hexchars' yields 22. 22 - 6 yields 16, an
error.

How'd I do coach? You got a job for me? In the Southwest if possible.
:-)
--
Joe Wright http://www.jw-wright.com
"Everything should be made as simple as possible, but not simpler."
--- Albert Einstein ---

[CBFalconer]

Joe Wright wrote:

CBFalconer wrote:

Joe Wright wrote:

... snip ...

/*
We will examine a printing hex character and determine its bin value.
Of course the numerics will translate directly. The alphas are a
special case as both 'a' and 'A' will evaluate to 10.
Input must be int within the ranges '0'..'9', 'A'..'F' and 'a'..'f'.

Some assumptions:

That '0',,'9', 'A'..'F' and 'a'..'f' are contiguous in the set.
'0'..'9' is guaranteed but the others are not. Oh well, they are
contiguous in ASCII and EBCDIC.
*/

/* Returns a value 0..15 for hex digits or -1 on failure. */
int h2b(int h) {

Why make any assumptions?

/* also useful for reverse conversions */
static char[] hexchars = "0123456789abcd efABCDEF";

/* Returns a value 0..15 for hex digits or -1 on failure. */
int h2b(int h)
{
char * s;

if (NULL == (s = strchr(hexchars , h))) h = -1;
else {
h = s - hexchars;
if (h > 15) h = h - 6;
}
if (h > 15) h = -1; /* Exercise - why this */
return h;
}

The string should be ..

static char hexchars[] = "0123456789abcd efABCDEF";

Exercise? There is a NUL at hexchars[22]. If h started off 0
strchr() will find the NUL and 'h = s - hexchars' yields 22.
22 - 6 yields 16, an error.

How'd I do coach? You got a job for me? In the Southwest if
possible. :-)

You did better than I did. No. How about one for me, in the
Northeast. :-[

At any rate my point was: Don't make unnecessary assumptions, even
though they may be convenient.

--
Chuck F (cb********@yah oo.com) (cb********@wor ldnet.att.net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home .att.net> USE worldnet address!

[Irrwahn Grausewitz]

CBFalconer <cb********@yah oo.com> wrote:

Joe Wright wrote:

CBFalconer wrote: <snip>

> /* also useful for reverse conversions */
> static char[] hexchars = "0123456789abcd efABCDEF";
>
> /* Returns a value 0..15 for hex digits or -1 on failure. */
> int h2b(int h)
> {
> char * s;
>
> if (NULL == (s = strchr(hexchars , h))) h = -1;
> else {
> h = s - hexchars;
> if (h > 15) h = h - 6;
> }
> if (h > 15) h = -1; /* Exercise - why this */
> return h;
> }

<snip> Exercise? There is a NUL at hexchars[22]. If h started off 0
strchr() will find the NUL and 'h = s - hexchars' yields 22.
22 - 6 yields 16, an error.

<snip>

My 2ct:

if ( h && (s = strchr( hexchars, h )) )
/*...*/

is a well-known idiom for retrieving the position of a non-null
character in a test string with strchr. Alternatively memchr
could be used, as suggested by Chris Torek elsethread.
Regards
--
Irrwahn Grausewitz (ir*******@free net.de)
welcome to clc : http://www.ungerhu.com/jxh/clc.welcome.txt
clc faq-list : http://www.faqs.org/faqs/C-faq/faq/
acllc-c++ faq : http://www.contrib.andrew.cmu.edu/~a...FAQ-acllc.html

[Herrcho]

Hi nrk~ Thanks for your comments. really good for my learning C.

i corrected my code.. Could you check it out ?

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

int lowercase(char c)
{
static const char *uppercase = "ABCDEFGHIJKLMN OPQRSTUVWXYZ";
static const char *lowercase = "abcdefghijklmn opqrstuvwxyz";

char *cptr;
cptr = strchr(uppercas e, c);
if ( cptr ) return lowercase[cptr - uppercase];
return c;
}

int char_to_num(cha r c)
{
char *k = "abcdef";

if (c >= '0' && c <= '9')
return c - '0';
else
return strchr(k, lowercase(c)) - k + 10;
}

int isxdigit2(char c)
{
static const char *hexalpha = "abcdefABCD EF";

if ( (c >= '0' && c <= '9') || (c && strchr(hexalpha , c)) )
return 1;
else
return 0;
}

unsigned htoi(char *c)
{
int sum = 0;

if ( c[0] == '0' && (c[1] == 'x' || c[1] == 'X') )
c += 2;
while (*c && isxdigit2(*c) )
{
sum = (sum * 16) + char_to_num(*c) ;
++c;
}
if ( *c ) {
fprintf(stdout, "Invalid hex digit %c in string\n", *c);
sum = 0;
}
return sum;
}

int main()
{
char *c = "0Xca";
printf("%u", htoi(c));

return 0;
}
/*************** *************** *************** *************** *****

it runs well , but i have a couple of questions..

what's the difference when i change 'stderr' to 'stdout' in the below.

fprintf(stderr, "Invalid hex digit %c in string\n", *c);

it doesn't make any difference in my machine.

and in isxdigit2 function, when i remove 'static' keyword ,

from 'static const char *hexalpha = "abcdefABCD EF"; '
to ' const char *hexalpha = "abcdefABCD EF"; '

what's the difference ? i know what static means in C, but

i don't see why 'static' needs in isxdigit2 function.

[CBFalconer]

Herrcho wrote:

.... snip ...
what's the difference when i change 'stderr' to 'stdout' in the below.

fprintf(stderr, "Invalid hex digit %c in string\n", *c);
stderr and stdout are different files, but usually (not always)
connected to the 'terminal' by default. You can control the
destination for stdout in most OSs with redirection. If that is
done using stderr allows the error messages to remain visible on
the terminal.

and in isxdigit2 function, when i remove 'static' keyword ,

from 'static const char *hexalpha = "abcdefABCD EF"; '
to ' const char *hexalpha = "abcdefABCD EF"; '

what's the difference ? i know what static means in C, but
i don't see why 'static' needs in isxdigit2 function.

The first line loads hexalpha once at program startup. The second
loads it every time the function isxdigit2 is called, and
generates extra code to do so.

--
Chuck F (cb********@yah oo.com) (cb********@wor ldnet.att.net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home .att.net> USE worldnet address!

[Irrwahn Grausewitz]

he*********@kor net.net (Herrcho) wrote:
<snip>

i corrected my code.. Could you check it out ?
Now that you've called for it ... ;-)
/*************** *************** *************** *************** *************
#include <stdio.h>
#include <string.h>

int lowercase(char c) To be consistent with the standard library functions, I suggest
to use int for character arguments.{
static const char *uppercase = "ABCDEFGHIJKLMN OPQRSTUVWXYZ";
static const char *lowercase = "abcdefghijklmn opqrstuvwxyz"; Nitpick:
static const char const *...
;-) char *cptr; Nit:
const char *cptr;
cptr = strchr(uppercas e, c);
if ( cptr ) return lowercase[cptr - uppercase];
return c;
} int char_to_num(cha r c) See above.{
char *k = "abcdef"; Nit:
static const char const *k = "abcdef"
if (c >= '0' && c <= '9')
return c - '0';
else
return strchr(k, lowercase(c)) - k + 10;
}
And you better make sure to exclusively feed hexdigit characters
to the char_to_num function (in the call in main below you do, of
course).
int isxdigit2(char c) See above, plus: identifiers beginning with 'is' followed by a
lowercase letter are reserved for future library extensions;
is_xdigit2 would be OK, though.{
static const char *hexalpha = "abcdefABCD EF"; Nit: static const char const *...
if ( (c >= '0' && c <= '9') || (c && strchr(hexalpha , c)) )
return 1;
else
return 0;
} unsigned htoi(char *c) unsigned htoi( const char *c ){
int sum = 0; Since you are going to return the value of sum, let it match
the return type:
unsigned int sum = 0;

I would use unsigned long (or maybe even size_t) as return type,
anyway.
if ( c[0] == '0' && (c[1] == 'x' || c[1] == 'X') )
c += 2;
while (*c && isxdigit2(*c) )
{
sum = (sum * 16) + char_to_num(*c) ;
++c;
}
if ( *c ) {
fprintf(stdout, "Invalid hex digit %c in string\n", *c);
sum = 0; I'm not sure if altering the conversion result in this case is
the most sensible thing to do.
}
return sum;
}

int main()
{
char *c = "0Xca"; const char *c = ...
printf("%u", htoi(c));

return 0;
}
/*************** *************** *************** *************** *****

it runs well , I assume you didn't compile with maximum warning level... ;-)
but i have a couple of questions..
what's the difference when i change 'stderr' to 'stdout' in the below.

fprintf(stderr , "Invalid hex digit %c in string\n", *c);

it doesn't make any difference in my machine.
The stdout and stderr streams may be associated with different
files/devices.

<OT>
Imagine your program being invoked from a unixish command shell
like this:

foo 2>foo_stderr_lo g

</OT>

There may also be a difference in buffering, see C99 7.19.3:

7 [...] As initially opened, the standard error stream is
not fully buffered; the standard input and standard output
streams are fully buffered if and only if the stream can
be determined not to refer to an interactive device.
and in isxdigit2 function, when i remove 'static' keyword ,

from 'static const char *hexalpha = "abcdefABCD EF"; '
to ' const char *hexalpha = "abcdefABCD EF"; '

what's the difference ? i know what static means in C, but
i don't see why 'static' needs in isxdigit2 function.

The static storage-class specifier really isn't needed in this
case, but it's sensible to use it: since the pointer isn't
changed throughout program execution, one single instance of
it is sufficient for the code at hand. Thus it's logical to
declare it static at function scope and let it being initialized
during program startup, rather than creating and destroying an
automatic instance each and every time the function is called or
returns, respectively. As mentioned above, the most pedantic
declaration is:

static const char const *hexalpha = "....";

You could also drop the pointer variable completely and put
the string literal in the call to strchr, but IMHO that would
just add to obfuscation and wouldn't have any real advantages.

HTH
Regards
--
Irrwahn Grausewitz (ir*******@free net.de)
welcome to clc : http://www.ungerhu.com/jxh/clc.welcome.txt
clc faq-list : http://www.faqs.org/faqs/C-faq/faq/
acllc-c++ faq : http://www.contrib.andrew.cmu.edu/~a...FAQ-acllc.html

[nrk]

Herrcho wrote:

Hi nrk~ Thanks for your comments. really good for my learning C.

i corrected my code.. Could you check it out ?

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

int lowercase(char c)
{
static const char *uppercase = "ABCDEFGHIJKLMN OPQRSTUVWXYZ";
static const char *lowercase = "abcdefghijklmn opqrstuvwxyz";

char *cptr;
cptr = strchr(uppercas e, c);
if ( cptr ) return lowercase[cptr - uppercase];
return c;
}

int char_to_num(cha r c)
{
char *k = "abcdef";

static const char *hexalpha = "abcdef";

See Irrwahn and CBF's answers for why static might be a good idea. Also,
try to give meaningful names for long-lived variables such as this one.
Calling it "k" is not useful at all. Calling it hexalpha ostensibly
provides some idea of what you're trying to achieve.
if (c >= '0' && c <= '9')
return c - '0';
else
return strchr(k, lowercase(c)) - k + 10;
}
See Irrwahn's point about this function. I hit the post button in my
original reply and then thought of the issue Irrwahn brought up. It is a
good point. While in your current code, char_to_num is guaranteed to
receive only a valid lowercase hex digit, on your next cut-n-paste of this
"working" char_to_num, you may get a rather nasty surprise if that
pre-condition is not satisfied. So, here's a char_to_num that fails
gracefully:

int char_to_num(cha r c) {
static const char *lhexalpha = "abcdef";
char *cptr;

if ( c >= '0' && c <= '9' ) return c - '0';
else if ( c && (cptr = strchr(lhexalph a, c)) )
return 10 + cptr - hexalpha;
return -1; /* Negative return indicates failure */
}

Keep in mind that this char_to_num doesn't handle uppercase hex digits.
That's fine in the current context, but I would probably add a comment to
the function to avoid a surprise when I cut-n-paste this code somewhere
else later on.

int isxdigit2(char c)
{
static const char *hexalpha = "abcdefABCD EF";

if ( (c >= '0' && c <= '9') || (c && strchr(hexalpha , c)) )
I didn't elaborate on the (c && strchr(hexalpha , c)) part. But if you
didn't give it much thought, see CBF's post upthread with his little
"exercise" to see why this is important.
return 1;
else
return 0;
}

unsigned htoi(char *c)
{
int sum = 0;

If you're returning unsigned, why not make sum match that return?
unsigned sum = 0;
if ( c[0] == '0' && (c[1] == 'x' || c[1] == 'X') )
c += 2;
while (*c && isxdigit2(*c) )
{
sum = (sum * 16) + char_to_num(*c) ;
++c;
}
if ( *c ) {
fprintf(stdout, "Invalid hex digit %c in string\n", *c);
Error messages are better off in the standard error stream. I don't have my
K&R1 with me (it's thousands of miles away right now, and I definitely do
miss it. If it weren't out of my student budget, I would definitely buy
another copy :-( ), but I am sure it explains standard input (stdin),
standard output (stdout) and standard error (stderr), the three streams
that are available to you at the start of your program.
sum = 0;
}
return sum;
}

int main()
{
char *c = "0Xca";
printf("%u", htoi(c));

return 0;
}
/*************** *************** *************** *************** *****

it runs well , but i have a couple of questions..

what's the difference when i change 'stderr' to 'stdout' in the below.

fprintf(stderr, "Invalid hex digit %c in string\n", *c);

it doesn't make any difference in my machine.

See Irrwahn's answer. On typical systems, both stdout and stderr are tied
to your terminal by default. However, if you're on a Unixish system try
out his suggestion and view the contents of foo_stderr_log (or the file you
redirect stderr to) to see the difference between my version and yours.
and in isxdigit2 function, when i remove 'static' keyword ,

from 'static const char *hexalpha = "abcdefABCD EF"; '
to ' const char *hexalpha = "abcdefABCD EF"; '

what's the difference ? i know what static means in C, but

i don't see why 'static' needs in isxdigit2 function.

It is not needed, but is probably a good idea (excellent answers from others
tell you why, and there's nothing I can add).

-nrk.

--
Remove devnull for email

[nrk]

Irrwahn Grausewitz wrote:

he*********@kor net.net (Herrcho) wrote:
<snip>

i corrected my code.. Could you check it out ?

Now that you've called for it ... ;-)

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

int lowercase(char c)

To be consistent with the standard library functions, I suggest
to use int for character arguments.

{
static const char *uppercase = "ABCDEFGHIJKLMN OPQRSTUVWXYZ";
static const char *lowercase = "abcdefghijklmn opqrstuvwxyz";

Nitpick:
static const char const *...

Nitpick, ITYM:
static const char * const ...

Here, and in other places as well :-)

-nrk.

--
Remove devnull for email