Velocity Reviews > Hex to int

# Hex to int

rajash@thisisnotmyrealemail.com
Guest
Posts: n/a

 12-02-2007
Thanks to everyone for interesting discussions. To make the group
happy, my next listing has int main and headers declared!!

Here is my solution to Exercise 2.3.

// write htoi, hex to int

#include<stdio.h>

int main(int c, char **v)
{
int x=-1, htoi();
if(c>1)
x=htoi(v[1]);
if(x>=0)
printf("%d\n", x);
else
printf("Unspecified error.\n");
}

int htoi(char *s)
{
char *t=s;
int x=0, y=1;
if(*s=='0' && (s[1]=='x' || s[1]=='X'))
t+=2;
s+=strlen(s);
while(t<=--s) {
if('0' <= *s && *s <= '9')
x+=y*(*s - '0');
else if('a' <= *s && *s <= 'f')
x+=y*(*s - 'a' + 10);
else if('A' <= *s && *s <= 'F')
x+=y*(10 + *s - 'A');
else
return -1; /* invalid input! */
y<<=4;
}
return x;
}

Flash Gordon
Guest
Posts: n/a

 12-02-2007
http://www.velocityreviews.com/forums/(E-Mail Removed) wrote, On 02/12/07 22:01:
> Thanks to everyone for interesting discussions. To make the group
> happy, my next listing has int main and headers declared!!

If your only reason for doing it is to keep the group happy then you
don't understand the issues and should reread what people have told you.
Either that or choose something other than programming.

Also you have *not* included all the required headers.

> Here is my solution to Exercise 2.3.
>
> // write htoi, hex to int
>
> #include<stdio.h>

The space shortage ended some years back, so it will be easier to read
if you use some
#include <stdio.h>

> int main(int c, char **v)

Conventionally the parameters are called argc and argv. Although other
names are legal it makes it easier for everyone if you use the
conventional names.

> {
> int x=-1, htoi();

Very bad style on two counts.
1) You have not used the prototype style of declaration so the compiler
cannot check the number and type of parameters passed to htoi.
2) It is generally considered better to not declare the functions
locally but globally, since they are visible globally. So it would be
better to put "int htoi(char *s);" at file scope or define htos prior to
main.

> if(c>1)
> x=htoi(v[1]);
> if(x>=0)
> printf("%d\n", x);
> else
> printf("Unspecified error.\n");

How about returning the value from main that the return type promisses
will be returned?

> }
>
> int htoi(char *s)
> {
> char *t=s;
> int x=0, y=1;

Why not try using more than one character for variable names? It makes
it *far* easier to read if you use meaningful names.

> if(*s=='0' && (s[1]=='x' || s[1]=='X'))
> t+=2;
> s+=strlen(s);

You need string.h for strlen, without it your program invokes undefined
behaviour and could fail on some implementations, for example if size_t
(the return type of strlen) is 64 bits but int is only 32 bits.

> while(t<=--s) {
> if('0' <= *s && *s <= '9')
> x+=y*(*s - '0');
> else if('a' <= *s && *s <= 'f')
> x+=y*(*s - 'a' + 10);

The letters are not guaranteed to be consecutive and one some systems
they are not.

> else if('A' <= *s && *s <= 'F')
> x+=y*(10 + *s - 'A');
> else
> return -1; /* invalid input! */
> y<<=4;

None of the above handles overflow properly. Overflow of signed types
invokes undefined behaviour and might not (on some implementations will
not) do what you expect.

> }
> return x;
> }

--
Flash Gordon

Keith Thompson
Guest
Posts: n/a

 12-02-2007
(E-Mail Removed) writes:
> Thanks to everyone for interesting discussions. To make the group
> happy, my next listing has int main and headers declared!!
>
> Here is my solution to Exercise 2.3.
>
> // write htoi, hex to int
>
> #include<stdio.h>
>
> int main(int c, char **v)

The traditional names for the parameters to main() are argc and argv.
Using different names is legal, but a very bad idea; it just makes

> {
> int x=-1, htoi();

Putting a function declaration inside a function definition is legal,
but not a good idea. A function declaration with empty parentheses
says that the function takes a fixed but unspecied number and type(s)
of arguments.

Either declare htoi() with a full prototype at file scope (outside main()):

int htoi(char *s);

or move the full definition of htoi() above the definition of main(),
so you don't need a separate declaration.

> if(c>1)
> x=htoi(v[1]);
> if(x>=0)
> printf("%d\n", x);
> else
> printf("Unspecified error.\n");

Error messages are normally printed to stderr, not stdout.

You could detect the specific error of invoking main() with no
arguments. Consider something like this:

if (argc <= 1) {
fprintf(stderr, "Usage: %s arg\n", argv[0]);
exit(EXIT_FAILURE);
}

> }
>
> int htoi(char *s)

Consider declaring this static, since it's not used from any other
translation units.

> {
> char *t=s;
> int x=0, y=1;
> if(*s=='0' && (s[1]=='x' || s[1]=='X'))
> t+=2;
> s+=strlen(s);
> while(t<=--s) {
> if('0' <= *s && *s <= '9')

You can use the isdigit() function for this.

> x+=y*(*s - '0');
> else if('a' <= *s && *s <= 'f')
> x+=y*(*s - 'a' + 10);
> else if('A' <= *s && *s <= 'F')
> x+=y*(10 + *s - 'A');

You're assuming that the numeric codes for the letters 'a'..'f' and
'A'..'F' are contiguous. This is not guaranteed, and there are
character sets where the alphabet is not contiguous (though in the one
example of this that I know of, 'a'..'f' and 'A'..'F' happen to be
contiguous). Consider using the isxdigit() function.

> else
> return -1; /* invalid input! */
> y<<=4;
> }
> return x;
> }

These are mostly superficial points. I'd take a closer look at the
algorithm, but your use of meaningless single-character identifiers

--
Keith Thompson (The_Other_Keith) <(E-Mail Removed)>
Looking for software development work in the San Diego area.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

Ben Bacarisse
Guest
Posts: n/a

 12-03-2007
(E-Mail Removed) writes:

> Here is my solution to Exercise 2.3.

<snip>

I'll comment only on the thing not yet pointed out...

> int htoi(char *s)
> {
> char *t=s;
> int x=0, y=1;
> if(*s=='0' && (s[1]=='x' || s[1]=='X'))
> t+=2;
> s+=strlen(s);
> while(t<=--s) {

This loop condition is likely to invoke undefined behaviour. Can you
see why?

--
Ben.

vippstar@gmail.com
Guest
Posts: n/a

 12-03-2007
On Dec 3, 3:11 pm, Ben Bacarisse <(E-Mail Removed)> wrote:
> (E-Mail Removed) writes:
> > Here is my solution to Exercise 2.3.

>
> <snip>
>
> I'll comment only on the thing not yet pointed out...
>
> > int htoi(char *s)
> > {
> > char *t=s;
> > int x=0, y=1;
> > if(*s=='0' && (s[1]=='x' || s[1]=='X'))
> > t+=2;
> > s+=strlen(s);
> > while(t<=--s) {

>
> This loop condition is likely to invoke undefined behaviour. Can you
> see why?

If strlen(s) equals 0.

Ben Bacarisse
Guest
Posts: n/a

 12-03-2007
(E-Mail Removed) writes:

> On Dec 3, 3:11 pm, Ben Bacarisse <(E-Mail Removed)> wrote:
>> (E-Mail Removed) writes:
>> > Here is my solution to Exercise 2.3.

>>
>> <snip>
>>
>> I'll comment only on the thing not yet pointed out...
>>
>> > int htoi(char *s)
>> > {
>> > char *t=s;
>> > int x=0, y=1;
>> > if(*s=='0' && (s[1]=='x' || s[1]=='X'))
>> > t+=2;
>> > s+=strlen(s);
>> > while(t<=--s) {

>>
>> This loop condition is likely to invoke undefined behaviour. Can you
>> see why?

>
> If strlen(s) equals 0.

No. It can invoke UB when strlen(s) != 0 and it may not invoke UB in
some cases when strlen(s) == 0. Anyway, the question was partly
rhetorical. I think people learn better when they think, so I rather
hoped that my question would remain unanswered (at least for some while).

--
Ben.

RoS
Guest
Posts: n/a

 12-03-2007
In data Mon, 03 Dec 2007 13:11:05 +0000, Ben Bacarisse scrisse:
>rajash@thisisnotmyrealemail writes:
>
>> Here is my solution to Exercise 2.3.

><snip>
>
>I'll comment only on the thing not yet pointed out...
>
>> int htoi(char *s)
>> {
>> char *t=s;
>> int x=0, y=1;
>> if(*s=='0' && (s[1]=='x' || s[1]=='X'))
>> t+=2;
>> s+=strlen(s);
>> while(t<=--s) {

>
>This loop condition is likely to invoke undefined behaviour. Can you
>see why?

is it because if s point to "" => strlen(s)==0; (s+=0)==s and --s
point to memory not allowed to change (or read)?

Ben Bacarisse
Guest
Posts: n/a

 12-03-2007
RoS <(E-Mail Removed)> writes:

> In data Mon, 03 Dec 2007 13:11:05 +0000, Ben Bacarisse scrisse:
>>rajash@thisisnotmyrealemail writes:
>>
>>> Here is my solution to Exercise 2.3.

>><snip>
>>
>>I'll comment only on the thing not yet pointed out...
>>
>>> int htoi(char *s)
>>> {
>>> char *t=s;
>>> int x=0, y=1;
>>> if(*s=='0' && (s[1]=='x' || s[1]=='X'))
>>> t+=2;
>>> s+=strlen(s);
>>> while(t<=--s) {

>>
>>This loop condition is likely to invoke undefined behaviour. Can you
>>see why?

>
> is it because if s point to "" => strlen(s)==0; (s+=0)==s and --s
> point to memory not allowed to change (or read)?

s pointing to "" and/or strlen(s) being zero has nothing to do with
it. Just consider a simple call like htoi("20") and work out what has
to happen for the while loop to stop.

Whenever I see a while loop, I negate the test in my head as ask "is
this condition safe?". When there is code after the loop you can
check that that code makes sense in an environment where the loop
condition is false (not significant in this case).

It gets a little messy when there are assignment operators (and
equivalents) in the test but it is still worth doing. In languages
like C, you have to scan the body for breaks, gotos and returns but
that is simple enough.

--
Ben.

rajash@thisisnotmyrealemail.com
Guest
Posts: n/a

 12-03-2007
Ben Bacarisse wrote:
> (E-Mail Removed) writes:
>
> > Here is my solution to Exercise 2.3.

> <snip>
>
> I'll comment only on the thing not yet pointed out...
>
> > int htoi(char *s)
> > {
> > char *t=s;
> > int x=0, y=1;
> > if(*s=='0' && (s[1]=='x' || s[1]=='X'))
> > t+=2;
> > s+=strlen(s);
> > while(t<=--s) {

>
> This loop condition is likely to invoke undefined behaviour. Can you
> see why?

No.

James Kuyper
Guest
Posts: n/a

 12-04-2007
(E-Mail Removed) wrote:
> Ben Bacarisse wrote:
>> (E-Mail Removed) writes:
>>
>>> Here is my solution to Exercise 2.3.

>> <snip>
>>
>> I'll comment only on the thing not yet pointed out...
>>
>>> int htoi(char *s)
>>> {
>>> char *t=s;
>>> int x=0, y=1;
>>> if(*s=='0' && (s[1]=='x' || s[1]=='X'))
>>> t+=2;
>>> s+=strlen(s);
>>> while(t<=--s) {

>> This loop condition is likely to invoke undefined behaviour. Can you
>> see why?

>
> No.

For a string not prefixed by "0x" or "0X", what happens to s if t==s at
the time the loop condition is evaluated?