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
Received on 2016-06-21