ares_channel -> ares_channel_t *: don't bury the pointer

[bradh352]

ares_channel is defined as typedef struct ares_channeldata *ares_channel;. The problem with this, is it embeds the pointer into the typedef, which means an ares_channel can never be declared as const as if you write const ares_channel channel, that expands to struct ares_channeldata * const ares_channel and not const struct ares_channeldata *channel.

We will now typedef ares_channel_t as typedef struct ares_channeldata ares_channel_t;, so if you write const ares_channel_t *channel, it properly expands to const struct ares_channeldata *channel.

We are maintaining the old typedef for API compatibility with existing integrations, and due to typedef expansion this should not even cause any compiler warnings for existing code. There are no ABI implications with this change. I could be convinced to keep existing public functions as ares_channel if a sufficient argument exists, but internally we really need make this change for modern best practices.

This change will allow us to internally use const ares_channel_t * where appropriate. Whether or not we decide to change any public interfaces to use const may require further discussion on if there might be ABI implications (I don't think so, but I'm also not 100% sure what a compiler internally does with const when emitting machine code ... I think more likely ABI implications would occur going the opposite direction).

FYI, This PR was done via a combination of sed and clang-format, the only manual code change was the addition of the new typedef :)

Fix By: Brad House (@bradh352)

[dimbleby]

cf #96.

fwiw I continue to maintain a fork that scatters some const around (so as to generate better rust bindings) and welcome this step in that direction.

[bradh352]

cf #96.

fwiw I continue to maintain a fork that scatters some const around (so as to generate better rust bindings) and welcome this step in that direction.

Now that I'm completely reworking the internals of c-ares to modernize the codebase, I think there is a stronger argument these days to accomplish this. Also, your fork is likely impossible to maintain due to my recent not-so-insignificant changes :)

We're also scanning with sonarcloud, and I can point to that to strengthen the case:
https://sonarcloud.io/project/issues?resolved=false&types=CODE_SMELL&id=c-ares_c-ares&open=AYuXU1Gj8hZUwkk7Xj3S

SonarCloud issues the warning based on a violation of the MISRA-C standard. I have a long-term goal of getting to zero SonarCloud reported issues (without simply marking them as false positives or won't fix). There will likely be a few instances that this isn't technically possible, but that's the goal.

[dimbleby]

your fork is likely impossible to maintain due to my recent not-so-insignificant changes :)

the changes in my fork are so minimal - it really is just a scattering of const on the interface, and then propagating that until the compiler is happy - that so far even your more invasive changes have only been a modest amount of work for me.

So while this is a +1, as in that older issue I would not ask or expect you to give that too much weight.