3
\$\begingroup\$

I need to read a value from a text file and put it into a predefined type, in this particular case mode_t. This typedef can have a variety of types underlying it, so what is the portable way to read such a value and assign it to a variable of the predefined type.

Since you have no idea how large the type is, I was starting under the assumption that you should read using strtoimax/strtoumax and then range check somehow. What I came up with "works" (seemingly) but looks ugly to my eye.

For example:

static mode_t parse_mode ( const char * s ) {
    char *       e = NULL;
    uintmax_t    u;
    uintmax_t    x;
    mode_t       m;
    int          c;

    if ( s == NULL) goto error_no_value;
    if (*s == '\0') goto error_no_value;
    if (*s == '-' ) goto error_negative;
    errno = 0;
    u = strtoumax(s, &e, 8);              /* value is in octal */
    if (errno) goto error_range;
    if (e == s) goto error_bogus;
    c = *e;
    if (c && !isspace(c)) goto error_bogus;   /* WS after is OK */

    /* now the question part -- better way??? */
         if (sizeof(mode_t) == 1) x = UINT8_MAX;
    else if (sizeof(mode_t) == 2) x = UINT16_MAX;
    else if (sizeof(mode_t) == 4) x = UINT32_MAX;
    else if (sizeof(mode_t) == 8) x = UINT64_MAX;
    else goto error_what_size;
    if (u > x) goto error_too_big;

    m = (mode_t)u;
    return m;
error_no_value: fprintf(stderr, "Error: no value\n");    return (mode_t)-1;
error_negative: fprintf(stderr, "Error: negative\n");    return (mode_t)-1;
error_range:    fprintf(stderr, "Error: range\n");       return (mode_t)-1;
error_bogus:    fprintf(stderr, "Error: bogus\n");       return (mode_t)-1;
error_what_size:fprintf(stderr, "Error: what size\n");   return (mode_t)-1;
error_too_big:  fprintf(stderr, "Error: too big\n");     return (mode_t)-1;
}

[I've totally glossed over the question of whether or not the type is signed or unsigned and just assumed unsigned here]

PS, yes, I have "secret knowledge" that no valid mode is likely to exceed 0177777; please ignore that as it is not necessarily true for other predefined types of its ilk.

PPS, junk like time_t which might not even be integral is not a part of this question

EDIT Added in the error goop. Also, note that all leading whitespace has been skipped prior to this being called.

\$\endgroup\$
  • 1
    \$\begingroup\$ Please post your code unchanged. Now that you omitted some parts the code is not longer valid c code and thus off-topic. \$\endgroup\$ – t3chb0t Jul 23 '18 at 17:22
3
\$\begingroup\$

An alternative would be to store the value, then test whether any truncation occured:

m = u;
if (m != u) goto error_too_big;

I think some of the single-character variables (e, u, c, etc.) could be given better names. The next person to modify the code (which could be Future You) will pick it up more quickly if it has meaningful variable names.

\$\endgroup\$
2
\$\begingroup\$

strtou...() allows a leading '-'

It looks like OP wants to fail negative values and does catch many potential ones. if (*s == '-' ) is insufficient detection for inputs like " -123". strtou...() curiously allows a '-' sign, simply negating the unsigned value, so that is no help. Instead, code could first find the first non-white-space.

if ( s == NULL) goto error_no_value;
if (*s == '\0') goto error_no_value;
while (isspace((unsigned char) *s)) s++;
if (*s == '-' ) goto error_negative;

OP may want to allow input like "-0", yet leave that to another day. Assume that is invalid.

isspace(char)) pitfall

isspace(int x) is well defined for all values in the unsigned char range and EOF. As char may have an offending negative value, consider the below instead which also allows for multiple trailing white space.

// c = *e;
// if (c && !isspace(c)) goto error_bogus;
while (isspace((unsigned char) *e)) e++;
if (*e) goto error_bogus;

better way??

If we know the target type is some unsigned type, simply compare to (mode_t) -1. It is well defined and always the maximum value.

#define UMAX ((mode_t) -1)

if (u > UMAX) goto error_too_big;

Values that are too big should consider setting errno for the calling code's use and setting to the max value.

if (u > UMAX) {
  m = max;
  errno = ERANGE;
  goto error_too_big;
}

If the type may be some signed type, highly portable code is not truly possible. With sizeof (mystery_type) == sizeof(int32_t), we can conclude that MYSTERY_TYPE_MAX <= 0x7FFFFFFF, yet we cannot absolutely conclude MYSTERY_TYPE_MAX == 0x7FFFFFFF, even if it is highly probable. These are implementation defined issues and portable code uses defines to drive that. With nominally portable code, using compares, as in OP's code, is OK with various signed types. Be mindful that intN_t types are semi-optional - although quite common.

// Use fixed width types
if (sizeof m == sizeof(int32_t)) max = INT32_MAX;
...

// or use standard types
if (sizeof m == sizeofint)) max = INT_MAX;
...
\$\endgroup\$
  • \$\begingroup\$ You might want to put the while(isspace()) loop before the test for '\0' - a space-only input is as invalid as an empty input, I think. \$\endgroup\$ – Toby Speight Jul 23 '18 at 17:05
  • \$\begingroup\$ @TobySpeight Could go there, yet OP's code catches that later with if (e == s) goto error_bogus; \$\endgroup\$ – chux Jul 23 '18 at 17:10
  • \$\begingroup\$ I added a comment that all leading WS has already been skipped prior to this being called. Also this routined does not need to worry about trailing WS as that is also handled elseweher. Also, don't want to accept "-0" (is there a reason you might want to?) \$\endgroup\$ – John Hascall Jul 23 '18 at 20:16
  • \$\begingroup\$ When I use isspace() with anything but an int argument I get a compiler warning about non-int array index. Not sure how to reconcile this with your suggestion. \$\endgroup\$ – John Hascall Jul 23 '18 at 20:20
  • \$\begingroup\$ @JohnHascall Changing your code once answers arrive typically results in a roll-back or DV. Consider amending See After you ask your question \$\endgroup\$ – chux Jul 23 '18 at 20:21
0
\$\begingroup\$
/* now the question part -- better way??? */
       if (sizeof(mode_t) == 1) x = UINT8_MAX;
  else if (sizeof(mode_t) == 2) x = UINT16_MAX;
  else if (sizeof(mode_t) == 4) x = UINT32_MAX;
  else if (sizeof(mode_t) == 8) x = UINT64_MAX;

The maximum size for any unsigned integer is 2^n-1 for n bits. We know there are 8 bits in a byte (sizeof returns the number of bytes), and 2^n = 1 << n.

so the result is simply:

x = (1ull << (sizeof m * CHAR_BIT) - 1)
\$\endgroup\$
  • \$\begingroup\$ I meant for the other 1 - the one you're shifting: (1ull << (sizeof m * CHAR_BIT) - 1). Oh, and we don't know there are 8 bits in a char, so this fixes that error, too. \$\endgroup\$ – Toby Speight Jul 23 '18 at 17:02
  • 1
    \$\begingroup\$ @TobySpeight Note:, if UINT8_MAX is defined, then CHAR_BIT == 8 must be true. Still good to avoid naked magic numbers. \$\endgroup\$ – chux Jul 23 '18 at 17:52
  • 1
    \$\begingroup\$ Is there a danger that (1ull << (sizeof m * CHAR_BIT) will overflow? \$\endgroup\$ – John Hascall Jul 23 '18 at 20:21
  • \$\begingroup\$ @JohnHascall 1ull << (sizeof m * CHAR_BIT) could overflow when sizeof m > sizeof (unsigned long long). Not likely today, maybe in 20 yrs. \$\endgroup\$ – chux Jul 23 '18 at 20:28
  • \$\begingroup\$ What about sizeof m == sizeof (unsigned long long)? Or sizeof x == sizeof m? \$\endgroup\$ – John Hascall Jul 23 '18 at 20:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.