troglobit / mdnsd

Jeremie Miller's original mdnsd
BSD 3-Clause "New" or "Revised" License
55 stars 35 forks source link

Fix parsing of A record from packet #56

Closed fzs closed 1 year ago

fzs commented 1 year ago

This is also a bug that has been long in existence. Which makes me wonder about two things. First, is the fact that the IP address is reversed in the cache expected behaviour, or is no-one using this so that it hasn't surfaced yet?

And secondly, what are the intended target systems of the library? Should it be usable on any (e.g. embedded) system, i.e. also non-POSIX systems? Because the original code did not use any network library function. This bug (I call it a bug) was introduced when switching from an int to an in_addr, which brought in the the arpa/inet.h header. So should I refrain from using POSIX functions like inet_ntop? I saw them used in mdnsd.c so I would think it is okay. Because if this is code to be potentially run on more than a PC, it is rather fixed to little endian systems.

This is one of the two issues I have with these convenience functions like net2long. They do the conversion manually instead of using standard methods like ntohl etc, and are therefore only working on little endian systems. On a big endian system this will break. The other is that I personally dislike that these functions have side effects on the buffer pointer passed in without mentioning that in the function name in any way,

troglobit commented 1 year ago

There's a huge backlog of refactoring needed to get this codebase endian safe. I've also noticed the ugly hacks that are net2long() and I support changing to use standard functions. In other projects I've had the policy of keeping addresses in network byte order in internal data structures, I think we can do that here as well.

The target of my fork is for mdnsd to run smoothly on any arch. that Linux or *BSD supports, where Linux is tier one. Personally I will probably use mdnsd on Arm 32/64-bit and possibly even RISC-V. I used to work for a company that had PowerPC (P1010/P1020 and T1023 SoC:s), but that is being phased out from most orgs. now.

I hope that answers your questions.