Re: functypes: recv/send argument definitions

From: Timothe Litt <litt_at_acm.org>
Date: Mon, 26 Sep 2022 11:57:32 -0400

On 26-Sep-22 11:14, Daniel Stenberg via curl-library wrote:
> Hello!
>
> In my new PR[*], I am ripping out the old checks from configure and
> cmake that
> try to figure out what argument and return types recv and send want.
> The new
> system instead a) defaults to POSIX and b) offers functypes.h that can
> provide
> overrides for system that don't use the POSIX types.
>
> The reason for this is of course that the checks are very brute-force
> and can
> end up taking a long time and for all non-cmake/configure builds we
> already
> have the types provided anyway.
>
> This change removes crufty logic and simplifies the approach. I think
> it feels
> cleaner.
>
>  BUT
>
> There is an obvious risk that this change will break the build on systems
> outside of our autobuilds and CI setups. I don't think we can avoid
> it, but I
> still think this is a worthwhile change. Fixing the build will be easy
> for
> whoever runs into it and will in a typical case consist of adding
> lines to
> functypes.h in the same style as other platforms already have them.
>
> (The old check also checks select() types, but since no current code
> actually
> use those types the new system does not care about those.)
>
> Comment here or in the PR!
>
> PR: https://github.com/curl/curl/pull/9592
>
This seems to be an unnecessary change; nothing is currently broken.

I'm not a cmake expert, but configure is almost by definition a library
of "brute force" and ugly code.  It's function is to avoid having
builders edit .h and make files...so this PR is essentially a step
backwards for whatever non-POSIX platforms the tests support.

If curl goes down this path, where does it stop?  I think a fair
argument could be made that all of configure/cmake could be replaced by
"a few simple manual edits to a default config.h and Makefiles". 
(Though they'd evolve to be more than a few and messy...)  While I don't
think you want to go all the way there, what test do you have in mind
for similar proposals in the future?  Is there a standard for how much
"brute force" or "ugliness" justifies creating work for some users?

With respect to performance: with configure  I use a cache file (and
recommend it - see --cache-file); this means that the test compiles
aren't run except when the cache is rebuilt, which is rare.  I
assume/hope that cmake has a similar facility.

Breaking something for the sake of a little less ugly code doesn't seem
like the right thing to do.

Rather, it seems to be a solution in search of - or creating - a problem.

If this code was consuming a lot of maintenance effort or an uncacheable
significant performance issue, I'd be more sympathetic.  However, I'd
look for solutions that don't create a burden for users.

Removing the checks for select() types seems reasonable since no-one
uses the result.  There probably are more tests with no consumers that
could be removed; most project accumulate them...

Timothe Litt
ACM Distinguished Engineer
--------------------------
This communication may not represent the ACM or my employer's views,
if any, on the matters discussed.

-- 
Unsubscribe: https://lists.haxx.se/listinfo/curl-library
Etiquette:   https://curl.se/mail/etiquette.html

Received on 2022-09-26