0

For socket programming, there are two basic structures to deal with addresses: struct sockaddr_in and sockaddr. According to man, "the only purpose of this structure [sockadrr] is to cast the structure pointer passed in addr in order to avoid compiler warnings" Every manual, snippet of code on books or programmer assumes whenever a function says it takes a struct sockaddr* you can cast your struct sockaddr_in* to that type with ease and safety.

Example from man bind

struct sockaddr_in myaddr;
int s;

myaddr.sin_family = AF_INET;
myaddr.sin_port = htons(3490);
inet_aton("63.161.169.137", &myaddr.sin_addr.s_addr);

s = socket(PF_INET, SOCK_STREAM, 0);
bind(s, (struct sockaddr*)&myaddr, sizeof(myaddr));

The problem is this casting violates MISRA C++ 2008 Rule 5-2-7 violation: An object with pointer type shall not be converted to an unrelated pointer type, either directly or indirectly.

What I want to know is, to overcome this problem (I am sure there is tons of people working with sockets and MISRA) is always necessary to justify the deviation of the rule? Is there actually no alternative to this casting?

Related question explaining what the casting to sockaddr is done:

Socket Programming, Casting sockaddr_in to sockaddr. Why?

casting between sockaddr and sockaddr_in

Typecasting sockaddr structures

Javi
  • 1
  • 4
  • 2
    Please don't tag both C and C++, unless the question is explicitly using both languages. Your code is written in *one* language, and that's the language you should tag. This is really crucial since C and C++ are two *very* different languages. It's more confusing since you use MISRA **C++** but the code you show is more **plain C** rather than C++. – Some programmer dude Sep 22 '21 at 06:42
  • `(struct sockaddr*)myaddr` is nonsense, invalid syntax and a bug. It won't pass compilation either. Please post a _copy/pase of the real code_, not something artificial you wrote down just now. – Lundin Sep 22 '21 at 06:55
  • Question is tag C++ now. You are right, is more plain C rather than C++ but it is a very simple snippet of the code that I believe is enough to explain the question. – Javi Sep 22 '21 at 07:57
  • 1
    I would argue that the problem is that you copy-pasted code from a C program, instead of adapting it. When "translating" form one language to another, even if they seem similar, then direct translations usually doesn't turn out well. And copy-pasting almost never works. A better solution is to read the C code, understand what it does, and then rewrite it using C++ instead. So for example you don't need `struct` when defining variables. And more importantly, and related to the MISRA issue, you should not use C-style casts (like `(struct sockaddr*)`). – Some programmer dude Sep 22 '21 at 08:11
  • However, MISRA is *extremely* strict, and the rule seems to disallow the the only way to handle address-structures, since the structures really are unrelated and that a cast (like `reinterpret_cast(&myaddr)`) really is needed for the code to be correct. There seems to be a clash between MISRA requirements, and correct code. – Some programmer dude Sep 22 '21 at 08:13
  • But as far as I know, there is not other way to deal with sockets even in C++. The code is written by me from scracth, I understand it perfectly. The point is the MISRA rule disallow a cast (same with reinterpret_cast) but it seems there is no other way to work with sockets. – Javi Sep 22 '21 at 08:32
  • Like I said, there's a clash between either working code or MISRA compliance. If MISRA is mandatory, could you abstract out the low-level socket code into a component or library which doesn't have to be covered by MISRA? Perhaps you need to bring this up with your superiors to decide how to continue? – Some programmer dude Sep 22 '21 at 08:48
  • I don't think I can have a component not covered by MISRA, so like you are saying, it seems to be a higher-level decision and not a programer's one. Since I am not a socket expert, I had doubts about my code approach clashing with MISRA, but I read more and more about sockets plus your opinion, it seems clear to me now that is an unsolvable issue. Thank you very much. – Javi Sep 22 '21 at 09:00
  • @Someprogrammerdude No, this has nothing to do with C style casts at all. The rule simply bans converting from one struct type to a completely different struct type, which is undefined behavior in C and C++ both. – Lundin Sep 22 '21 at 12:54
  • @Lundin I never claimed it dd, only said that C-style casts should be avoided. – Some programmer dude Sep 22 '21 at 12:58
  • @Someprogrammerdude Well you incorrectly assumed that a cast was needed for the code to turn correct. `reinterpret_cast` wouldn't solve anything, it would still be strict aliasing UB in C and C++ both. The warning could as well come from a good compiler like gcc, as a MISRA checker. – Lundin Sep 22 '21 at 13:06
  • Your second link has the answer: it is undefined behaviour, and MISRA is correct to flag it – Caleth Sep 22 '21 at 13:06
  • The summary seems to be: this lib was designed by some MS "talents" who don't know the struct compatibility rules of C nor C++ but assume that you use some non-conforming MS compiler to compile the lib. – Lundin Sep 22 '21 at 13:09
  • @Lundin This "lib" (or rather what we now consider the "standard" networking API) was designed about 40 years ago. – Some programmer dude Sep 22 '21 at 13:10
  • @Lundin It's not just windows, POSIX has exactly this API too – Caleth Sep 22 '21 at 13:11
  • @Caleth MS copied the BSD networking stack (initially rather verbatim), where this API was introduced around 40 years ago. – Some programmer dude Sep 22 '21 at 13:12
  • @Someprogrammerdude oh, turns out I was looking at code using lwIP on linux and saw these casts – Caleth Sep 22 '21 at 13:24
  • Unrelated to the question, you might want to check out this: [Is MISRA-C useful outside safety-critical and embedded programming?](https://software.codidact.com/posts/279077) – Lundin Sep 22 '21 at 13:34

3 Answers3

1

As told upon reading rule 5.2.7, the error comes from attempting a cast between incompatible types. It has nothing to do with C or C++ casts - using reinterpret_cast would have been just as bad. You are simply doing a fishy conversion between two different structure types which is undefined behavior in C and C++ both.

It's been ages since I used Windows sockets but as far as I can tell sockaddr isn't the slightest compatible with sockaddr_in. I could be mistaken, but it would seem that the tool is simply saying "bug here" and the solution is to fix the bug?

Otherwise, if they are indeed equivalent in terms of memory layout, then in C you could have solved this by creating a union of the two struct types, initialize one type and pass the other to the function, also known as "type punning". This isn't allowed in C++ though. Note that winsock was designed for C and not C++.

The C++ solution would mean that you either have to memcpy one struct instance into the other (rather fishy practice) or write some serialize/de-serialize routine (slow).

Or you could just use the actual type that the function asks for and all problems will go away...

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • 2
    The initial members of `sockaddr_in` are the same as for `sockaddr` (`sockaddr_in` is basically a `sockaddr` structure, with some extra members added at the end). So a pointer to `sockaddr_in` can be converted to a pointers to `sockaddr` without breaking anything. – Some programmer dude Sep 22 '21 at 12:55
  • @Someprogrammerdude Not according to this: https://learn.microsoft.com/en-us/windows/win32/winsock/sockaddr-2. In order to type pun by "common initial sequence", you need a union with the two structs inside, but C compilers barely support that feature and it's not allowed in C++. So the tool correctly states that this is UB, in both C and C++. – Lundin Sep 22 '21 at 12:57
  • Btw let me add for completeness that designing mission-critical software using C++ and Windows is very likely utter madness. – Lundin Sep 22 '21 at 12:59
  • Okay, they are not truly CIS compatible, but they are the exact same size, and the important member is the initial "family" member which tells the kind of structure (together with their size, think `AF_LOCAL`). And these structures have existed since before C was standardized, using this exact cast. You can't find any low-level networking code without such a cast. IPv4 requires the `sockaddr_in` structure, and functions only accept the "base" `sockaddr` structure (or rather a pointer to it). There's just no way around it. Which means MISRA can never be used for "standard" networking code. – Some programmer dude Sep 22 '21 at 13:07
  • @Someprogrammerdude It rather means that "standard" networking code cannot be compiled in C or C++. This UB has nothing to do with MISRA. – Lundin Sep 22 '21 at 13:10
  • @Lundin an implementation is permitted to define the behaviour of specific cases of undefined (by the standard) behaviour – Caleth Sep 22 '21 at 13:13
  • @Caleth Sure, but if you are writing mission-critical software you shouldn't rely on some old non-standard slop, which compilers may turn into a wreck at any moment. In this case it's probably justified to do a hard copy of the data just to turn the code portable to C and C++. If you are using TCP/IP you already threw real-time performance out the window anyhow, so a little struct hard copy overhead is nothing to be concerned about. – Lundin Sep 22 '21 at 13:16
  • @Lundin The code is never going to be portable, because you are calling functions supplied by the implementation – Caleth Sep 22 '21 at 13:20
  • @Lundin One problem is that you *can't* copy, because some network address structures are larger than the `sockaddr` structure (see `sockadd_local`). That's why functions like `bind` also needs the size of the structure as an argument.And as I mentioned, this have been in common use since before C was standardized, which means the initial C standard would have invalidated just about *all* of the infrastructure running the Internet at the time of standardization (of C). And continues to do so to this day, – Some programmer dude Sep 22 '21 at 13:22
  • @Caleth So it isn't part of POSIX then...? You just said it was. If some new *nix or Windows version pops up for some new CPU, then ideally the code should be portable and not break because it relies on UB. – Lundin Sep 22 '21 at 13:23
  • The "implementation is permitted to define the behavior of specific cases" loophole mentioned by @Caleth must be implemented by all C and C++ compilers unless they want to watch the whole of the Internet crash and burn. – Some programmer dude Sep 22 '21 at 13:24
  • @Lundin I was incorrect about these structs being in POSIX, but `winsock.h` is distributed with the windows sdk for msvc, and there's a popular embedded network stack that has equivalent definitions of these types (and needs this casting between them) – Caleth Sep 22 '21 at 13:27
  • @Someprogrammerdude As explained in the answer, C provides means to do type punning but this isn't how. One could use a union instead of a cast. And that's not even a problem with the socket lib but the caller code. Also "this lib has been rotten since the dawn of time" is a poor excuse, it's not like it's the only socket lib out there, there exists numerous TCP/IP stacks for embedded systems. Why desktop programmers have been too lazy to write a modern lib when they had 40 years to do so, I don't know. It's not like 40 years old network hardware is still allowed to be in use. – Lundin Sep 22 '21 at 13:29
  • And the fact that certain versions of gcc _will_ go haywire upon strict aliasing violations remains, particularly those versions around year 2000 when C99 was new. – Lundin Sep 22 '21 at 13:31
  • @Lundin - but those who look at Windows code and go *yuck* best not have a look at the Linux kernel. It's horrible! – Andrew Sep 25 '21 at 08:36
0

Example from man bind


myaddr.sin_family = AF_INET; myaddr.sin_port = htons(3490);
inet_aton("63.161.169.137", &myaddr.sin_addr.s_addr);

s = socket(PF_INET, SOCK_STREAM, 0); bind(s, (struct
sockaddr*)&myaddr, sizeof(myaddr)); ```

The problem is this casting violates MISRA C++ 2008 Rule 5-2-7
violation: An object with pointer type shall not be converted to an
unrelated pointer type, either directly or indirectly.

Correct. You are casting a pointer to struct sockaddr_in to a pointer to struct sockaddr. Castering to an unrelated type is generally a Bad Thing.

If you really need to do this, then you need to raise a deviation, not just to document that you are doing it, but to detail the mitigations that you will perform to demonstrate that your code is fit to use.

Andrew
  • 2,046
  • 1
  • 24
  • 37
0

From my experience with that kind of code (in MISRA context, too): one just adds proper exception to the linter and goes on with it. That kind of need to cast is generally a design flaw in the socket API every programmer has stumbled upon and not much cannot be done to avoid it (if anything).

What can (and probably should) be done though is to limit the number of those casts and cast only when passing to socket specific API functions, never earlier; in turn, this shall lead to limiting the number of socket API calls by abstracting them away as a workaround. But still, the typecast cannot be avoided, only their number and scope can (and should) be limited.

alagner
  • 3,448
  • 1
  • 13
  • 25
  • Thank you so much for your answer and for share your experience. It was my suspect, that is a flaw in the API but I couldn't find people struggling with socket programming. – Javi Sep 27 '21 at 05:29