Should I manually typecast to a higher type if there is an evident overflow possibility?

Question

I am learning C and in one of the tasks I am asked to use 1000 * getuid() + 0 to generate the semaphore name for sem_open. There will be multiple semaphores and the last digit (+ 0) is used to differentiate them.

Code snippet:

#define SEM_NAME_LENGTH 24
#define SEM_NAME 1000 * getuid() + 0

...

  char sem_name[SEM_NAME_LENGTH];

  /* convert the sem number (as defined in spec) to a string */
  if (sprintf(sem_name, "%u", SEM_NAME) < 0) {
    return -1;
  }

  sem_id = sem_open(sem_name, O_CREAT, S_IRUSR | S_IWUSR, 0);

It is evident that SEM_NAME can exceed unsigned int, because getuid can return UINT_MAX, and if we multiply that by 1000...

My first thought was to change the sprintf format to "%llu", but then I get a warning:

format specifies type 'unsigned long long' but the argument has type 'unsigned int'

Which means that compilers are still considering the expression to be an unsigned int or __uid_t.

Looks like I have the following possibilities:

A. Manually typecast to unsigned long long:

#define SEM_NAME (unsigned long long) 1000 * getuid() + 0

B. Define the name as variable:

unsigned long long sem_name = 1000 * getuid() + 0;

C. Check for overflows / not accept uids higher than UINT_MAX/1000 (bad)

I am also quite surprised that compilers (gcc, clang) don't detect the problem themselves. I am using -std=gnu99 -Wall -Wextra -Wstrict-prototypes -pedantic.

@EOF Rather `#define SEM_NAME (1000ull * getuid() + 0)` I daresay. — cadaniluk, May 06 '16 at 12:04
@Downvoter: In general, yes. However, in this case `SEM_NAME` is only used as a function argument, so it doesn't matter. — EOF, May 06 '16 at 12:06
Actually it seems that you are solving a non-existing problem. Uid can not be too big, on most systems UID_MAX is 60000. If you want to be sure to avoid overflow, I'd propose solution C. Anyway, when using integer arithmetic (especially multiplication) you simply have to use some assumptions about actual values stored in your variables. Otherwise sizes of your data types will grow fast. — Marian, May 06 '16 at 13:03
@Marian, I agree that this would hardly cause problems in the real world, but by digging deeper I learn more. — amq, May 06 '16 at 13:08
An alternative solution could be `sprintf(sem_name, "%u%03u", getuid(), 0);` — Ian Abbott, May 06 '16 at 13:54

score 1 · Answer 1 · edited May 23 '17 at 12:16

C Check for overflows

As needed, pedantic code checks for potential range issues.

uid_t id = getuid();
if (id > UINT_MAX/1000 || id < 0) Handle_OutOfRange(id);
sprintf(sem_name, "%u", (unsigned) (id * 1000u));

More practically...

Interestingly code use "%u", even though the type was not know to be unsignedas getuid() returns type uid_t which can be signed or unsigned and of a width different than unsigned.

// potential undefined behavior - UB
sprintf(sem_name, "%u", SEM_NAME)

Following OP's A approach of promoting to a wide type, code could use the following of going to the widest type. IMO, why go to long long/unsigned long long? Go the the widest integer type intmax_t/uintmax_t.

uid_t id = getuid();
sprintf(sem_name, "%jd", (intmax_t) id * 1000);

In general, do not recommend OP's #define which hides a function call in a macro SEM_NAME that appears to be a constant. Alternative suggested

// #define SEM_NAME 1000 * getuid() + 0
// sprintf(sem_name, "%u", SEM_NAME)

#include <stdint.h>
#define SEM_NAME(id)  (INTMAX_C(1000) * (id) + 0)

uid_t id = getuid();
sprintf(sem_name, "%jd", SEM_NAME(id));

Should I manually typecast to a higher type if there is an evident overflow possibility?

1 Answers1