comments on curl

[aep]

first of all this is super useful.
probably a reason for me to make sure we have a website with disoverable modules soon.

https://github.com/zx-project/zx/blob/master/modules/curl/zz.toml#L3

i mean, why not just "curl". you're the first one to make a curl module, so just grab the name ;)

https://github.com/zx-project/zx/blob/master/modules/curl/tests/curl.zz#L15

any chance to use the err module here? In general let me know how you think errors can be improved.

https://github.com/zx-project/zx/blob/master/modules/curl/src/easy.zz#L30

why are these nessesary? this looks like something that needs improving in zz?

https://github.com/zx-project/zx/blob/master/modules/curl/src/easy.zz#L68

this should require

where buffer_lenght <= len(buffer)

for safety.

https://github.com/zx-project/zx/blob/master/modules/curl/src/easy.zz#L110

should require where nullterm(string)

etc

https://github.com/zx-project/zx/blob/master/modules/curl/src/options.zz#L3

this looks tedious. why are these necessary?

[jwerle]

first of all this is super useful.
probably a reason for me to make sure we have a website with disoverable modules soon.

hell yeah

/modules/curl/zz.toml@master#L3
i mean, why not just "curl". you're the first one to make a curl module, so just grab the name ;)

🤷‍♂️ I'll drop the zx

/modules/curl/tests/curl.zz@master#L15
any chance to use the err module here? In general let me know how you think errors can be improved.

yeah, absolutely!

/modules/curl/src/easy.zz@master#L30
why are these nessesary? this looks like something that needs improving in zz?

I couldn't get setting libcurl::* functions to a struct to work with the function type system so I had to wrap them

If you try setting

escape: libcurl::curl_easy_escape

you'll get:

/home/werle/repos/zx-project/zx/modules/curl/src/easy.zz:19:5: error: incompatible pointer types initializing 'zxcurl_prototype_EscapeFunction' (aka 'const char *(*)(void *const, const char *const, const int)') with an expression of type 'char *(CURL *, const char *, int)' (aka 'char *(void *, const char *, int)') [-Werror,-Wincompatible-pointer-types]
    curl_easy_escape,.perform = 
    ^~~~~~~~~~~~~~~~
1 error generated.
clang "target/test/z

/modules/curl/src/easy.zz@master#L68
this should require

where buffer_lenght <= len(buffer)

for safety.

got it

/modules/curl/src/easy.zz@master#L110
should require where nullterm(string)

etc

👍

/modules/curl/src/options.zz@master#L3
this looks tedious. why are these necessary?

try to give a cleaner looking experience for curl options:

curl::options::WRITEDATA vs CURLOPT_WRITEDATA

it is definitely tedious and probably unnecessary

[jwerle]

@aep when I try to name the project to curl I get naming collisions with the actual libcurl symbols such as curl_easy_init() because the curl module exports the easy module with an init() function: curl::easy::init() which maps directly to the symbol curl_easy_init() which causes the conflict. Something like zetzit/zz#56 could solve this or I can just continue using a unique name

[aep]

Hmmm how would that solve it.

Or how would it be different from just not calling it curl

[jwerle]

calling it curl with the current symbol layout directly conflicts with the symbol names defined in curl.h .

curl_easy_init(), a libcurl symbol, and curl::easy::init() a ZZ symbol, both conflict when ZZ is compiled to C. The solution for using the curl name is to change the symbol layout so it doesn't collide with libcurl

[aep]

yes, i just dont understand how this relates to zetzit/zz#56

calling it something_curl or something::curl results in the same symbol name

in rust people often rust something.rs so maybe curl_zz ?

[jwerle]

I have very limited experience in rust, just c/c++. I only bring zetzit/#56 up because namespaces of some kind could solve this with out having to prefix curl with zx or postfix with _zz. As I'm still getting used to ZZ and learning all the conventions you've brought over from rust I may propose silly things like this or may not be very clear in my questioning/explanations!

Happy to do here what you think will be best for the future

[aep]

ah ok now i understand why you're bringing it up, thanks :)

symbol namespacing do exist, and C++ and rust are using them. zetz intentionally does not use them, because C doesnt have them.

It would only work when linking zetz with other zetz code, which is against the basic idea of zetz always emitting code that can be used directly in other projects without wrappers.

I would suggest to name the curl wrapper something not "curl" then :)

[jwerle]

hell yeah I am glad that made sense. Yeah changing the name is the easiest thing to do!

any suggestions? curl_zz?

[aep]

sounds good!

[jwerle]

🙌