Re: wget2 | Abandon use of gcc attribute 'nonnull' from API (#200)

[@rockdaboot]

Tim Rühsen commented:


Good you raise this question @getjff 

`wget.h` has it's own definitions of gcc's attributes because we can't just 
export gnulib symbols (gnulib is just a portability source code layer). So 
instead of `G_GNUC_WGET_NONNULL` we have `WGET_GCC_NONNULL(_ALL)`. The comment 
you cite above needs to be changed. (If you are not familiar with Gitlab / 
Github / git workflows, creating an MR just for this tiny change seems to be a 
good starter.)

Then see the comment at 
https://gitlab.com/gnuwget/wget2/-/blob/master/include/wget/wget.h#L134.
I think it would be nice to use this attribute when building with scan-build, 
clang's static analyzer to find bugs / misuses in our own code. But if people 
use libwget we should have NULL checks for input pointer parameters to prevent 
any crashes in libwget itself. This possibly needs a reconsideration of the 
current API - e.g. it's a good practice to return an error and have pointer 
parameters for output/results. Originally, I decided against this to have less 
code overhead and thus increased code maintainability. We have to reconsider 
this possibly before the first release.

So this turns out to be not a 'junior' task :-)

One thing we could do is to only define `WGET_GCC_NONNULL(_ALL)` when a certain 
define is set. E.g. put `#ifdef STATIC_ANALYZE_LIBWGET` around the defines. And 
use `CFLAGS=... -DSTATIC_ANALYZE_LIBWGET` when building wget2 in the scan-build 
CI image.

@getjff Try to join https://riot.im/app/#/room/#wget:talk.go7box.xyz, a Matrix 
channel to talk about wget/wget2 topics. You meet me and @darnir there and can 
ask any questions - with a shorter response time than here.

-- 
Reply to this email directly or view it on GitLab: 
https://gitlab.com/gnuwget/wget2/-/issues/200#note_634764803
You're receiving this email because of your account on gitlab.com.