CODEWITHSUNDEEP
ccastingoverflow
amq
amq

Reputation: 505

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

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.

Upvotes: 0

Views: 108

Answers (1)

chux
chux

Reputation: 153517

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

Upvotes: 1

Related Questions