Hi,

I found that this has been discussed already back in 2007 and Daniel seemed to be positive about it, but then the discussion ended without a visible decision one way or the other, so I am picking this up again.

http://thread.gmane.org/gmane.comp.web.curl.library/13979

The fact that these three types are defined as void suppresses compiler warnings if pointers to them are mixed among each other or with other pointers.

I recently stumbled over this as the maintainer of the TclCurl package n openSUSE when I received a bug report about it segfaulting when the multi interface is used. It turned out that there are some places in TclCurl where accidentally a CURLM pointer gets passed to a function that expects a pointer to an internal structure of TclCurl, leading to memory corruption and eventually triggering the segfault.

After I had found and fixed this, I changed the definition of these types to be opaque structures:

typedef struct CURL   CURL;
typedef struct CURLM  CURLM;
typedef struct CURLSH CURLSH;

This revealed a few more cases of CURLM being passed instead of a TclCurl structure and one case of a CURLSH pointer variable that was accidentally declared as CURL.

Therefore I strongly suggest to change the definition of these types to something that does allow typecheckig. I am neutral on whether this is done using dummy names like I did above or by using the actual internal names as it was suggested in the old thread, as long as it is done at all.

Using dummy names has the advantage of not revealing additional details about libcurl internals, but people can read the source code anyway, so it doesn't really hide anyting. I think using the internal names would have the advantage of extending the typechecking into libcurl itself, because functions that take one of these types would not need an explicit cast to the internal type, but I haven't checked this thoroughly.

AFAICS neither variant of the change would constitute an API or ABI change on relevant platforms (unless old Crays are still considered relevant) and for proper uses of libcurl. It should only produce new compiler warnings or errors in cases where libcurl is used improperly, be it accidentially or intentionally.

cu
Reinhard

-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html

Reply via email to