# K&R 2 exercise 2-3

Discussion in 'C Programming' started by Herrcho, Feb 4, 2004.

1. ### HerrchoGuest

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++ )
{
if (!isxdigit2(c) )
{
printf("Wrong hexa number\n");
return 0;
}
c = char_to_num(c);
}

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..

Herrcho, Feb 4, 2004

2. ### Peter PichlerGuest

"Herrcho" <> wrote in message
news:...
> Hi~ i've studied C for a few months myself,

Good for you.

> 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

Excellent, finally someone who has actually shown some code! ;-)
Beware, many of my comments below are nits, but it is nice to learn the
good habits now before you'd need to unlearn the wrong ones, like myself.

> "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)

Identifiers starting with 'is' followed by a lowercase letter are reserved
by the C standard. Use something like is_xdigit2 instead.
By the way, there already is a macro isxdigit, you need to #include
<ctype.h>

> {
> if ( (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') || (c >= '0' && c <=

'9') )

This is not guaranteed to work on non-ASCII systems. The ranges 'a'..'f' and
'A'..'F' do not need to be consecutive. It is guaranteed about '0'..'9',
though.

> return 1;
> else
> return 0;
> }

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

Same comments as above. BTW, identifiers starting with 'to' are reserved
too.

> 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;

Where does the magic number 48 come from? ITYM return c - '0';

> }
> else
> {
> return 10 + (tolower2(c) - 'a');

Again, this only works if you are sure that 'a'..'f' is a consecutive set.

> }
> }
>
> int htoi(char *c)
> {
> int i, k, prefix = 0;
> size_t sum = 0;
>
> if (c[0] == '0' && tolower2(c[1]) == 'x')
> prefix = 1;

Are you sure that you have 2 valid characters in c? What happens when you
call htoi("7"), for example?
Try:

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

It's admittably a bit less elegant but safer. By the way, you could work on
an algorithm without the variable prefix. It's really simple, think about it
a little. All you need is to skip the first 2 characters of c...

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

"Hexa" as in "jinxed"? ;-)

> return 0;
> }
> c = char_to_num(c);
> }
>
> 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));

Err, no sir. %u in printf() expects unsigned int, but you provide it with
the result of htoi(), which returns int. This is strictly speaking an
undefined behaviour, although admittably I have yet to see a platform where
it does not work. In any case, use %d instead or change your htoi() to
return unsigned int.

> return 0;
> }

It looks OK, though overly complicated. You could use already available
macros isxdigit and tolower (both defined in ctype.h), but frankly you
should not even need them. And you certainly should not need your power2().

Think about it. What's 1986 in decimal?
In your algorithm, it's 1*10^3 + 9*10^2 + 8*10^1 + 1*10^0.
How about (((1)*10 + 9)*10 + 8)*10 + 6?

Now try converting it to a C program, for any (hexa)decimal number with any
number of digits.

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

Undefined behaviour. Your htoi() tries to alter the string it is given in
situ. With char c[] in main, the string literal "0xAB" get copied to a local
array c, which is OK. But when you declare c as char *, you pass a pointer
to the string literal to htoi()... oops!

> Thanks..

HTH,

Peter

Peter Pichler, Feb 4, 2004

3. ### nrkGuest

Herrcho wrote:

> 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."
>
>

First off, not a bad attempt. But the biggest problem is that you've
assumed ASCII character set. Read on for the complete review...

>

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

#include <string.h> /* 'coz I am gonna use strchr */

> int isxdigit2(int c)
> {
> if ( (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') || (c >= '0'
> && c <= '9') )

Here you assume 'a'-'f' and 'A'-'F' are in contiguous ascending order in the
character set. This need not be true (yes, even I wasn't happy to know
that). Note however that '0'-'9' are guaranteed to be in contiguous
ascending order by the standard.

So, what do we do? Simple, we keep a string with valid hex-alphabets and
see if the character to be checked occurs within that string:

static const char *hexalpha = "abcdefABCDEF";

if ( (c >= '0' && c <= '9') || (c && strchr(hexalpha, c)) )

> return 1;
> else
> return 0;
> }
>

Alternately, you could just use the isxdigit standard function, but that's
way less fun

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

Ouch!! Once again, you've not only assumed that 'A'-'Z' are contiguous
ascending, but also assumed that 'A'+32 == 'a', which need not be true at
all. You could either use the standard tolower, or:
static const char *uppercase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
static const char *lowercase = "abcdefghijklmnopqrstuvwxyz";

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

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

Hmmm... way too complicated... see below for why this isn't needed.

> int char_to_num(int c)
> {
> if (c >= '0' && c <= '9')
> {
> return c - 48;

Ouch!! Here you assumed '0' == 48, which need not be true. Why not:
return c - '0';

> }
> else
> {
> return 10 + (tolower2(c) - 'a');

Again, 'a'-'f' need not be contiguous ascending in the character set. What
static const char *hexalpha = "abcdef";
return 10 + (strchr(hexalpha, tolower2(c)) - hexalpha);

> }
> }
>
> int htoi(char *c)

Make that:
unsigned int htoi(char *c)

> {
> int i, k, prefix = 0;
> size_t sum = 0;

unsigned int sum;
>
> if (c[0] == '0' && tolower2(c[1]) == 'x')
> prefix = 1;
>
> for (i = (prefix == 1)? 2:0 ; c ;i++ )
> {
> if (!isxdigit2(c) )
> {
> printf("Wrong hexa number\n");
> return 0;
> }
> c = char_to_num(c);
> }
>
> for (k = (prefix == 1)? 2 : 0 ; k <= i-1 ; ++k )
> {
> sum += c[k] * power2(16, i-1-k);
> }
>
> return sum;
> }
>

You can simplify matters quite a bit if you recognize that proceeding
left-to-right in the string is the best thing that you can do. Simply
multiply whatever you have by 16 and add the next number in line, and voila
you have the correct conversion at the end.

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(stderr, "Invalid hex digit %c in string\n", *c);
sum = 0;
}
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 ??
>

Who shows what error?

-nrk.

> Thanks..

--
Remove devnull for email

nrk, Feb 4, 2004
4. ### nrkGuest

Peter Pichler wrote:

> "Herrcho" <> wrote in message
> news:...
>> Hi~ i've studied C for a few months myself,

>
> Good for you.
>
>> 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

>
> Excellent, finally someone who has actually shown some code! ;-)
> Beware, many of my comments below are nits, but it is nice to learn the
> good habits now before you'd need to unlearn the wrong ones, like myself.
>
>> "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)

>
> Identifiers starting with 'is' followed by a lowercase letter are reserved
> by the C standard. Use something like is_xdigit2 instead.
> By the way, there already is a macro isxdigit, you need to #include
> <ctype.h>
>
>> {
>> if ( (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') || (c >= '0' && c
>> <=

> '9') )
>
> This is not guaranteed to work on non-ASCII systems. The ranges 'a'..'f'
> and 'A'..'F' do not need to be consecutive. It is guaranteed about
> '0'..'9', though.
>
>> return 1;
>> else
>> return 0;
>> }

>
>

The indentation's ok, but he/she used tabs instead of spaces. To OP: You
should use spaces to indent code that you post on the usenet.

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

>
> Same comments as above. BTW, identifiers starting with 'to' are reserved
> too.
>
>> 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;

>
> Where does the magic number 48 come from? ITYM return c - '0';
>
>> }
>> else
>> {
>> return 10 + (tolower2(c) - 'a');

>
> Again, this only works if you are sure that 'a'..'f' is a consecutive set.
>
>> }
>> }
>>
>> int htoi(char *c)
>> {
>> int i, k, prefix = 0;
>> size_t sum = 0;
>>
>> if (c[0] == '0' && tolower2(c[1]) == 'x')
>> prefix = 1;

>
> Are you sure that you have 2 valid characters in c? What happens when you
> call htoi("7"), for example?

Any non-empty string will have valid values in c[0] and c[1]. Also, the
standard guarantees that '\0' != '0'

> Try:
>
> if (c[0] && c[0] == '0' && c[1] && (c[1] == 'x' || c[1] == 'X'))
> prefix = 1;
>
> It's admittably a bit less elegant but safer. By the way, you could work
> on an algorithm without the variable prefix. It's really simple, think
> about it a little. All you need is to skip the first 2 characters of c...
>
>> for (i = (prefix == 1)? 2:0 ; c ;i++ )
>> {
>> if (!isxdigit2(c) )
>> {
>> printf("Wrong hexa number\n");

>
> "Hexa" as in "jinxed"? ;-)
>
>> return 0;
>> }
>> c = char_to_num(c);
>> }
>>
>> 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));

>
> Err, no sir. %u in printf() expects unsigned int, but you provide it with
> the result of htoi(), which returns int. This is strictly speaking an
> undefined behaviour, although admittably I have yet to see a platform
> where it does not work. In any case, use %d instead or change your htoi()
> to return unsigned int.
>
>> return 0;
>> }

>
> It looks OK, though overly complicated. You could use already available
> macros isxdigit and tolower (both defined in ctype.h), but frankly you
> should not even need them. And you certainly should not need your
> power2().
>
> Think about it. What's 1986 in decimal?
> In your algorithm, it's 1*10^3 + 9*10^2 + 8*10^1 + 1*10^0.
> How about (((1)*10 + 9)*10 + 8)*10 + 6?
>
> Now try converting it to a C program, for any (hexa)decimal number with
> any number of digits.
>
>> //******************************************************************
>>
>> when i change char c[] to char *c in main(),
>> it shows error, why ??

>
> Undefined behaviour. Your htoi() tries to alter the string it is given in
> situ. With char c[] in main, the string literal "0xAB" get copied to a
> local array c, which is OK. But when you declare c as char *, you pass a
> pointer to the string literal to htoi()... oops!
>

Waah... I missed that one, didn't I? Tricksy little hobbitses...

-nrk.

>> Thanks..

>
> HTH,
>
> Peter

--
Remove devnull for email

nrk, Feb 4, 2004
5. ### Vijay Kumar R ZanvarGuest

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

#define MAX_STR 80

void
reverse ( char * str )
{
char *beg, *end;

beg = str;
end = str + strlen ( str ) - 1;

while ( beg < end )
{
*beg ^= *end ^= *beg ^= *end;
beg++;
end--;
}
return;
}

int
to_num ( char dig )
{
if ( !isxdigit ( dig ) )
{
fprintf ( stderr, "Not a hexadecimal number" );
exit ( EXIT_FAILURE );
}

if ( isalpha ( dig ) )
else
return dig - '0';
}

int
main ( void )
{
char str[MAX_STR];
long dec = 0, exp = 0;
char *ptr;

printf ( "Enter a hexadecimal number: " ); if ( !fgets ( str, MAX_STR,
stdin ) ) return EXIT_FAILURE; /* discard "0x" or "0X" */
if ( str[0] == '0' )
if ( tolower ( str[1] ) == 'x' )
memmove ( str, str+2, 3 );
puts ( str );

/* use function reverse(s) of exercise 1-19.c */
reverse ( str );
ptr = str;

while ( *ptr )
{
dec = dec + to_num ( *ptr ) * pow ( 16, exp );
exp++;
ptr++;
}

printf ( "Decimal: %ld\n", dec );
return EXIT_SUCCESS;
}

Vijay Kumar R Zanvar, Feb 4, 2004
6. ### Peter PichlerGuest

"nrk" <> wrote:
> Peter Pichler wrote:
> > "Herrcho" <> wrote:
> >>
> >> int htoi(char *c)
> >> {
> >> int i, k, prefix = 0;
> >> size_t sum = 0;
> >>
> >> if (c[0] == '0' && tolower2(c[1]) == 'x')
> >> prefix = 1;

> >
> > Are you sure that you have 2 valid characters in c? What happens when

you
> > call htoi("7"), for example?

>
> Any non-empty string will have valid values in c[0] and c[1]. Also, the
> standard guarantees that '\0' != '0'

Err... yes, but... mumbleohbuggerhumblegrumble... OK, I concede ;-)

Peter Pichler, Feb 4, 2004
7. ### nrkGuest

Vijay Kumar R Zanvar wrote:

Did you even care to test it once? Apart from being needlessly complicated,
reliant on a contiguous ascending alphabet character set, unnecessarily
using real arithmetic, causing gratuituous undefined behavior, it is also
hopelessly broken because fgets may leave '\n' in your buffer.

> #include <stdio.h>
> #include <stdlib.h>
> #include <ctype.h>
> #include <math.h>
> #include <string.h>
>
> #define MAX_STR 80
>
> void
> reverse ( char * str )
> {
> char *beg, *end;
>
> beg = str;
> end = str + strlen ( str ) - 1;

Disaster waiting to happen when str == "".

>
> while ( beg < end )
> {
> *beg ^= *end ^= *beg ^= *end;
> beg++;
> end--;

Needlessly cute. Try to use the language correctly before being "leet".
Whatever in the world is wrong with?:
*beg++ = *end--;
Do you object to it because it is more readable and demonstrably correct in
this context? Do you object to it because it is likely more efficient?

> }
> return;
> }
>
> int
> to_num ( char dig )
> {
> if ( !isxdigit ( dig ) )
> {
> fprintf ( stderr, "Not a hexadecimal number" );

Missing '\n'.

> exit ( EXIT_FAILURE );
> }
>
> if ( isalpha ( dig ) )

What if 'a' > 'b'? What if 'b' == 'a' + 3?

> else
> return dig - '0';
> }
>
>
> int
> main ( void )
> {
> char str[MAX_STR];
> long dec = 0, exp = 0;
> char *ptr;
>
> printf ( "Enter a hexadecimal number: " ); if ( !fgets ( str,
> MAX_STR,
> stdin ) ) return EXIT_FAILURE; /* discard "0x" or "0X" */
> if ( str[0] == '0' )
> if ( tolower ( str[1] ) == 'x' )
> memmove ( str, str+2, 3 );

Oh Joy!! Even more undefined behavior. Who told you that str+3 and str+4
will contain something valid? If str == "0x" or str == "0x\n", you end up
touching uninitialized memory.

> puts ( str );
>
> /* use function reverse(s) of exercise 1-19.c */
> reverse ( str );
> ptr = str;
>
> while ( *ptr )
> {
> dec = dec + to_num ( *ptr ) * pow ( 16, exp );
> exp++;
> ptr++;
> }
>
> printf ( "Decimal: %ld\n", dec );
> return EXIT_SUCCESS;
> }

Try to write unoptimised, "unleet", correct code before getting fancy. Try
to atleast test your code once before posting it [this one doesn't work for
one single valid case, except on stdin redirection and a non-standard file
with no ending newline that contains a valid hex number].

-nrk.

--
Remove devnull for email

nrk, Feb 5, 2004
8. ### nrkGuest

nrk wrote:

> Vijay Kumar R Zanvar wrote:
>

>
> Did you even care to test it once? Apart from being needlessly
> complicated, reliant on a contiguous ascending alphabet character set,
> unnecessarily using real arithmetic, causing gratuituous undefined
> behavior, it is also hopelessly broken because fgets may leave '\n' in
>
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <ctype.h>
>> #include <math.h>
>> #include <string.h>
>>
>> #define MAX_STR 80
>>
>> void
>> reverse ( char * str )
>> {
>> char *beg, *end;
>>
>> beg = str;
>> end = str + strlen ( str ) - 1;

>
> Disaster waiting to happen when str == "".
>
>>
>> while ( beg < end )
>> {
>> *beg ^= *end ^= *beg ^= *end;
>> beg++;
>> end--;

>
> Needlessly cute. Try to use the language correctly before being "leet".
> Whatever in the world is wrong with?:
> *beg++ = *end--;
> Do you object to it because it is more readable and demonstrably correct
> in this context? Do you object to it because it is likely more efficient?

Got carried away there. You should object because it is demonstrably
incorrect Obviously you need to swap the two values with a temporary
intermediary:

char temp = *beg;
*beg++ = *end;
*end-- = temp;

Taking a dose of my own medicine, I should've tested the darn thing before
posting.

Also, that memmove breaks your program for most valid inputs starting with
0x.

-nrk.

nrk, Feb 5, 2004
9. ### Joe WrightGuest

nrk wrote:
>
> nrk wrote:
>
> > Vijay Kumar R Zanvar wrote:
> >

> >
> > Did you even care to test it once? Apart from being needlessly
> > complicated, reliant on a contiguous ascending alphabet character set,
> > unnecessarily using real arithmetic, causing gratuituous undefined
> > behavior, it is also hopelessly broken because fgets may leave '\n' in
> >
> >> #include <stdio.h>
> >> #include <stdlib.h>
> >> #include <ctype.h>
> >> #include <math.h>
> >> #include <string.h>
> >>
> >> #define MAX_STR 80
> >>
> >> void
> >> reverse ( char * str )
> >> {
> >> char *beg, *end;
> >>
> >> beg = str;
> >> end = str + strlen ( str ) - 1;

> >
> > Disaster waiting to happen when str == "".
> >
> >>
> >> while ( beg < end )
> >> {
> >> *beg ^= *end ^= *beg ^= *end;
> >> beg++;
> >> end--;

> >
> > Needlessly cute. Try to use the language correctly before being "leet".
> > Whatever in the world is wrong with?:
> > *beg++ = *end--;
> > Do you object to it because it is more readable and demonstrably correct
> > in this context? Do you object to it because it is likely more efficient?

>
> Got carried away there. You should object because it is demonstrably
> incorrect Obviously you need to swap the two values with a temporary
> intermediary:
>
> char temp = *beg;
> *beg++ = *end;
> *end-- = temp;
>
> Taking a dose of my own medicine, I should've tested the darn thing before
> posting.
>
> Also, that memmove breaks your program for most valid inputs starting with
> 0x.
>
> -nrk.

Try this..

/*
Program: htol.c
Author: Joe Wright <>

Convert a hexadecimal char to its numeric equivalent.
Accommodate ASCII and EBCDIC. Maybe others.
*/

#include <stdio.h>

typedef unsigned char uchar;
typedef unsigned long ulong;

/*
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) {
int i, b = -1; /* Error code */
if ((i = h - '0') >= 0 && i < 10)
b = i;
else if ((i = h - 'a') >= 0 && i < 6)
b = i + 10;
else if ((i = h - 'A') >= 0 && i < 6)
b = i + 10;
return b;
}

ulong h2l(char *h) {
ulong l = 0;
int c;
while ((c = *h++) && (c == '0' || c == 'x' || c == 'X'))
;
if (c)
do {
l = l * 16 + h2b(c);
} while((c = *h++));
return l;
}

int main(int argc, char *argv[]) {
ulong ans = 0;
if (argc > 1)
ans = h2l(argv[1]);
printf("\t%lu\n", ans);
return 0;
}

--
Joe Wright http://www.jw-wright.com
"Everything should be made as simple as possible, but not simpler."
--- Albert Einstein ---

Joe Wright, Feb 5, 2004
10. ### CBFalconerGuest

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 = "0123456789abcdefABCDEF";

/* 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;
}

--
Chuck F () ()
Available for consulting/temporary embedded and systems.

CBFalconer, Feb 6, 2004
11. ### Chris TorekGuest

In article <news:>
CBFalconer <> writes:
>Why make any assumptions?
>
>/* also useful for reverse conversions */
>static char[] hexchars = "0123456789abcdefABCDEF";

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) +1 801 277 2603
Reading email is like searching for food in the garbage, thanks to spammers.

Chris Torek, Feb 6, 2004
12. ### peteGuest

Chris Torek wrote:
>
> In article <news:>
> CBFalconer <> writes:
> >Why make any assumptions?
> >
> >/* also useful for reverse conversions */
> >static char[] hexchars = "0123456789abcdefABCDEF";

>
> 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

pete, Feb 6, 2004
13. ### Joe WrightGuest

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 = "0123456789abcdefABCDEF";
>
> /* 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[] = "0123456789abcdefABCDEF";

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 ---

Joe Wright, Feb 6, 2004
14. ### CBFalconerGuest

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 = "0123456789abcdefABCDEF";
> >
> > /* 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[] = "0123456789abcdefABCDEF";
>
> 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 () ()
Available for consulting/temporary embedded and systems.

CBFalconer, Feb 7, 2004
15. ### Irrwahn GrausewitzGuest

CBFalconer <> wrote:
>Joe Wright wrote:
>> CBFalconer wrote:

<snip>
>> > /* also useful for reverse conversions */
>> > static char[] hexchars = "0123456789abcdefABCDEF";
>> >
>> > /* 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 ()
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/~ajo/docs/FAQ-acllc.html

Irrwahn Grausewitz, Feb 7, 2004
16. ### HerrchoGuest

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 = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
static const char *lowercase = "abcdefghijklmnopqrstuvwxyz";

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

int char_to_num(char 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 = "abcdefABCDEF";

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 = "abcdefABCDEF"; '
to ' const char *hexalpha = "abcdefABCDEF"; '

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

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

Herrcho, Feb 8, 2004
17. ### CBFalconerGuest

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 = "abcdefABCDEF"; '
> to ' const char *hexalpha = "abcdefABCDEF"; '
>
> 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 () ()
Available for consulting/temporary embedded and systems.

CBFalconer, Feb 8, 2004
18. ### Irrwahn GrausewitzGuest

(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 = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> static const char *lowercase = "abcdefghijklmnopqrstuvwxyz";

Nitpick:
static const char const *...
;-)
> char *cptr;

Nit:
const char *cptr;

> cptr = strchr(uppercase, c);
> if ( cptr ) return lowercase[cptr - uppercase];
> return c;
>}

>int char_to_num(char 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 = "abcdefABCDEF";

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_log

</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 = "abcdefABCDEF"; '
>to ' const char *hexalpha = "abcdefABCDEF"; '
>
>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

HTH
Regards
--
Irrwahn Grausewitz ()
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/~ajo/docs/FAQ-acllc.html

Irrwahn Grausewitz, Feb 8, 2004
19. ### nrkGuest

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 = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> static const char *lowercase = "abcdefghijklmnopqrstuvwxyz";
>
> char *cptr;
> cptr = strchr(uppercase, c);
> if ( cptr ) return lowercase[cptr - uppercase];
> return c;
> }
>
> int char_to_num(char 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;
> }

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(char c) {
static const char *lhexalpha = "abcdef";
char *cptr;

if ( c >= '0' && c <= '9' ) return c - '0';
else if ( c && (cptr = strchr(lhexalpha, 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 = "abcdefABCDEF";
>
> 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 = "abcdefABCDEF"; '
> to ' const char *hexalpha = "abcdefABCDEF"; '
>
> 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, Feb 8, 2004
20. ### nrkGuest

Irrwahn Grausewitz wrote:

> (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 = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
>> static const char *lowercase = "abcdefghijklmnopqrstuvwxyz";

> Nitpick:
> static const char const *...

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

Here, and in other places as well

-nrk.

--
Remove devnull for email

nrk, Feb 8, 2004