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

[Peter Pichler]

"Herrcho" <he*********@ko rnet.net> wrote in message
news:76******** *************** **@posting.goog le.com...

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;
}
Proper indentation would make your code easier to read.
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] ;i++ )
{
if (!isxdigit2(c[i]) )
{
printf("Wrong hexa number\n");
"Hexa" as in "jinxed"? ;-)
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));
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

[nrk]

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 = "abcdefABCD EF";

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 = "ABCDEFGHIJKLMN OPQRSTUVWXYZ";
static const char *lowercase = "abcdefghijklmn opqrstuvwxyz";

char *cptr = strchr(uppercas e, 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
you can do instead is:
static const char *hexalpha = "abcdef";
return 10 + (strchr(hexalph a, 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] ;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;
}

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]

Peter Pichler wrote:

"Herrcho" <he*********@ko rnet.net> wrote in message
news:76******** *************** **@posting.goog le.com...

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

Proper indentation would make your code easier to read.

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] ;i++ )
{
if (!isxdigit2(c[i]) )
{
printf("Wrong hexa number\n");

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

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

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

[Vijay Kumar R Zanvar]

How about this one?
#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 ) )
return tolower ( dig ) - 'a' + 10;
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;
}

[Peter Pichler]

"nrk" <ra*********@de vnull.verizon.n et> wrote:

Peter Pichler wrote:

"Herrcho" <he*********@ko rnet.net> 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... mumbleohbuggerh umblegrumble... OK, I concede ;-)

[nrk]

Vijay Kumar R Zanvar wrote:

How about this one?
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 ) )
return tolower ( dig ) - 'a' + 10;
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]

nrk wrote:

Vijay Kumar R Zanvar wrote:

How about this one?

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?

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.

[Joe Wright]

nrk wrote:

nrk wrote:

Vijay Kumar R Zanvar wrote:

How about this one?

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?

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 <jo********@ear thlink.net>

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

[CBFalconer]

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

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