On Tue, 21 Jun 2016 at 16:52, Daniel Stenberg wrote:
On Tue, 21 Jun 2016, Reinhard Max wrote:
After another glance over libcurl I think in addition to changing
the typedefs all uses of the typedef'ed names should be purged from
the implementation and replaced by the actual struct names.
This will be a larger change, but comes with the benefit of
simplifying the code by removing explicit typecasts while allowing
typechecking not only for the users of libcurl, but also throughout
the implementation.
We don't need to change them to gain that. We gain the type checks
for the library code the same way we gain it for external apps with
those forward declared structs. Those typedefs are used for libcurl
code as well.
Ok, but woudn't it be cleaner to have the internal code only use the
fully-declared version of the structs and external code only use the
opaque typecasts? And given that you are touching most of the affected
lines in your patch already doing this cleanup as well wouldn't even
make the patch much larger.
But we can happily remove a bunch of local declarations that are
made and then assigned the input argument just to convert them into
the internal struct pointers.
At least some of these existing assignments also contain typecasts,
which would break type checking again, even with the new typedefs, so
they should definitely be removed.
That said, with the new typedef setup there's no _need_ to use them
in the libcurl code but there's also no harm in doing so as I see
it.
Right, technically there is no need, but I still think it'd be cleaner
to have a clear cut between internal and external types and C
compilers don't seem to care if types differ between declaration and
definition of a function as long as they are compatible.
cu
Reinhard
-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html