HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Converting fraction notation in strings to a Rational data type

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
datarationaltypenotationfractionconvertingstrings

Problem

I've been messing around with a RationalInt type, using a simple
int-based structure for the time being. I've created code to convert
strings to the RationalInt type, but I'm not happy with it; it is too
convoluted for comfort (though this second rewrite isn't as bad as the
first rewrite, and the initial revision treated too many mal-formatted
fractions as valid).

Simple examples of valid fractions are:

  • 1



  • 1/3



  • -1 1/4



  • 1.5



The input code does not require the fraction representation to be
minimal. For example:

  • +6 17/3 is valid enough, but corresponds to 11 2/3 or 35/3.



The code allows arbitrary sequences of blanks and tabs (but not
newlines) between the components of a fraction, but only allows a sign at
the start. (That is, neither 6 +17/3 nor 6 17/-3 is a valid
fraction, but the 6 would be recognized as a valid integer, and hence
fraction; the end of the conversion would be the blank after the 6.)

However, the code ensures that values which exceed the range of the
int type (assumed to be 32-bit int) are not allowed. For example:

  • 1234567 192214/662391 is invalid because the exact fraction


155375261911 / 662391 cannot be represented with two 32-bit integers.

The code identifies the end of the 'subject string' which it recognizes
as a fraction, like the strtol() and related functions do. When it
returns an error, it sets errno to either EINVAL or ERANGE (but
does not set errno to zero, of course).

The function really under review is ri_scn() and the functions it
calls. It uses the ri_new() function, so that's included. It does
not use the rest of the RationalInt package.

The only external non-standard function used is chk_strtoi(). This is
based on the interface for strtol(), but limits the range of valid
values to the range of int (INT_MIN..INT_MAX), and checks that the
conversion was valid and reports that via the boolean return value.

rational.h (extract)

```
#ifndef RAT

Solution

Bug

Currently, if you try this input:

100000 1/100000


you will get this result:

1410065409 / 100000


The problem is with this overflow check in cvt_compound():

/* i, n, d are all valid integers, but can i + n/d be represented? */
if (i > (INT_MAX - d) / n)
    return seteor_return(eor, fs->d_end, -1, ERANGE);


The n and d need to be switched, like this:

/* i, n, d are all valid integers, but can i + n/d be represented? */
if (i > (INT_MAX - n) / d)
    return seteor_return(eor, fs->d_end, -1, ERANGE);

Code Snippets

100000 1/100000
1410065409 / 100000
/* i, n, d are all valid integers, but can i + n/d be represented? */
if (i > (INT_MAX - d) / n)
    return seteor_return(eor, fs->d_end, -1, ERANGE);
/* i, n, d are all valid integers, but can i + n/d be represented? */
if (i > (INT_MAX - n) / d)
    return seteor_return(eor, fs->d_end, -1, ERANGE);

Context

StackExchange Code Review Q#113158, answer score: 2

Revisions (0)

No revisions yet.