1

I have a system call like the following one:

int transfer(int handle, int direction, unsigned char *data, int length);

I have written the following two functions:

int input(int handle, void* data, int length)
{
    return transfer(handle, 1, static_cast<unsigned char*>(data), length);
}

int output(int handle, const void* data, int length)
{
    return transfer(handle, 0, static_cast<unsigned char*>(const_cast<void*>(data)), length);
}

I don't like the nested const_cast inside the static_cast, is there a way to perform the conversion from const void* to unsigned char* in a single step?

lornova
  • 6,667
  • 9
  • 47
  • 74
  • 9
    That cast sequence being ugly is a feature here, it is dangerous. – Mat Jul 09 '18 at 15:33
  • 7
    The only c++ cast that can remove `const` is `const_cast`, even `reinterpret_cast` (which lets you try to cast almost anything) won't let you cast away `const`. A single step cast that would work is a C cast. It would resolve to the same (or nearly) series of casts, but it would appear to be a single cast. I would recommend keeping the long series of casts. – François Andrieux Jul 09 '18 at 15:33
  • is actually _const correct your_ output function? – Moia Jul 09 '18 at 15:33
  • 1
    @Moia the system function `transfer` is quite ugly, but it doesn't modify the passed buffer if `direction` is `0` (output). – lornova Jul 09 '18 at 15:36
  • 1
    Well, reading your comments you have to adapt to something that is not well designed and that you cannot change. I think you have no other way to do, but using C-style cast or copying the `data` into a `unsigned char` buffer that you would pass to `transfer()` instead. You probably don't want to do that. – Shlublu Jul 09 '18 at 15:57
  • const in `const void* data` is only there so that you would not modify `data` inside of your function. But as you cast it away, the funciton signature might just as well be `int output(int handle, void* data, int length)`. Or better `int output(int handle, unsigned char* data, int length)`. – JHBonarius Jul 09 '18 at 16:30
  • @JHBonarius the `input` and `output` functions have the proper parameters, they exist for the purpose of hiding the ugly and bad signature of the `transfer` function. – lornova Jul 10 '18 at 09:07
  • @Wizard79 but that's only a mirage, as you immediately cast away the `const`. Externally you're just 'tricked' that there's a guarantee that the parameter you pass will not be modified. Like I said in my answer: what if `transfer` would actually modify the value? Then you could potentially have an unexplained data corruption, which could take a long time to debug. I would not cover up this signature by recasting everything, because you're misleading the user. Instead make a copy or so, like I propose in my answer. – JHBonarius Jul 10 '18 at 09:48
  • @JHBonarius it's not a mirage, it's a contract. The contract is respected as the buffer passed to `transfer` is not modified if `direction` is 0. `const` itself doesn't translate to any code, it is a contract itself. I think that your suggestion to make a copy is very bad: it has a performance and memory impact for no reason. – lornova Jul 10 '18 at 10:41
  • @Wizard79 That might be the case for now, but what if somebody modifies the code of `transfer` (or some comparable function in an similar situation). For instance now reordering the data in `data` for optimal communication: then you suddenly have a change in your original data. So, you might be convinced that you're doing the right thing, but along the way you're making a lot of assumptions. I've been debugging a lot of old code in my career, where designers were making such assumptions that turned up as a bug after x years. `const_cast` can be very dangerous. – JHBonarius Jul 10 '18 at 11:08
  • @JHBonarius if and when `transfer` will do something as dumb as modifying the output buffer, `output` will be changed accordingly. By the way, this is what happens inside glibc `write`: `const` qualifier for the passed buffer is silently dropped when passed to `syscall`. You have to trust the contract, the buffer will not be modified inside the kernel for this syscall. – lornova Jul 10 '18 at 11:19
  • Even contracts can changed over time. I've seen the bugs... In this case I'd expect the API supplier to provide suitable `transmit` and `receive` interfaces, instead of one ring to rule them all. By the way glibc is not C++. It's not C even, but often implemented in assembly. There's no `const` in assembly. – JHBonarius Jul 10 '18 at 11:38

2 Answers2

3

Using a C-style cast will generate identical assembly. As seen in Compiler Explorer:

//Source #1
int transfer(int handle, int direction, unsigned char *data, int length);
int input(int handle, void* data, int length)
{
    return transfer(handle, 1, static_cast<unsigned char*>(data), length);
}

int output(int handle, const void* data, int length)
{
    return transfer(handle, 0, static_cast<unsigned char*>(const_cast<void*>(data)), length);
}

//Source #2
int transfer(int handle, int direction, unsigned char *data, int length);
int input(int handle, void* data, int length)
{
    return transfer(handle, 1, (unsigned char*)data, length);
}

int output(int handle, const void* data, int length)
{
    return transfer(handle, 0, (unsigned char*)data, length);
}

//Assembly (both)
input(int, void*, int):
        mov     ecx, edx
        mov     rdx, rsi
        mov     esi, 1
        jmp     transfer(int, int, unsigned char*, int)
output(int, void const*, int):
        mov     ecx, edx
        mov     rdx, rsi
        xor     esi, esi
        jmp     transfer(int, int, unsigned char*, int)

So it's clear that simply using a C-style cast will solve your problem.

However, you shouldn't use a C-style cast

The reason for the verboseness of the C++ casts is to ensure that you aren't making mistakes. When a maintainer sees your code, it's important that they see the const_cast and the static_cast, because writing the code this way informs the reader that casting away the const-ness of the pointer is intentional and desired behavior. A code maintainer should see these casts, and presume that there was intent behind the code, instead of having to guess whether you knew that casting directly from const void* to unsigned char* would involve risking Undefined Behavior. Your example might not contain UB (since you specified that the contract of transfer is to treat data as read-only when direction is 0), but it's important that anyone else who needs to make changes to your code understands the deliberateness of your coding practices.

Community
  • 1
  • 1
Xirema
  • 19,889
  • 4
  • 32
  • 68
  • 1
    _However, you shouldn't use a C-style cast_ Disagree. If what you are doing with C++ style casts is effectively the same, then the code will be a lot clearer (and shorter) if you simply code up a C-style cast and be done with it. If the person reading the code has so little faith in you that he doesn't "trust" you, then there is a deeper problem (and you can always comment the code). – Paul Sanders Jul 09 '18 at 17:04
  • 1
    _The reason for the verboseness of the C++ casts is to ensure that you aren't making mistakes_ No, it is for expressiveness. The reason for the _different flavours_ of C++ casts is to (try to) ensure that you are not making mistakes. (Welcome to pedantry corner). – Paul Sanders Jul 09 '18 at 17:05
  • 1
    @PaulSanders "Clearer" and "Shorter" are not synonyms in this context. See [this answer](https://stackoverflow.com/a/103868/5241642) for a more comprehensive rationale for insisting on C++-style casts over C-style casts. – Xirema Jul 09 '18 at 17:20
  • I don't think I claimed that they were. And while that is a nice answer you link to there, I don't see it as being pertinent to the OP's specific question (because he is just casting pointers to primitive objects). – Paul Sanders Jul 09 '18 at 17:23
0

You don't want data to be un-const'd. What if transfer tried to modify it? It would be safer to have a local copy:

#include <cstring>
int output(int handle, const void* data, int length)
{
    unsigned char localData[length];
    std::memcpy(localData, data, length);
    return transfer(handle, 0, localData, length);
}

edit: reading through the comments, this is what Shlublu suggested...

JHBonarius
  • 10,824
  • 3
  • 22
  • 41
  • This answer is off topic, the question is "is there a way to perform the conversion from const void* to unsigned char* in a single step?". You are answering to "in this scenario it is correct to cast away const?" – lornova Jul 10 '18 at 10:44
  • By the way, there is absolutely nothing wrong in casting away `const` here: `transfer` will not modify the passed buffer if `direction` is 0. – lornova Jul 10 '18 at 10:48
  • @Wizard79 SO is full of XY problems. Very often the correct answer to a question is not what the OP asked. – JHBonarius Jul 10 '18 at 10:52