is assert() an appropriate substitute for return -1?

is assert() an appropriate substitute for return -1?

[Sam Roberts]

>From libnfnetlink.c:


 * On success, 0 is returned. On error, -1 is returned and errno is set
 * appropiately.
 */
int nfnl_callback_register(struct nfnl_subsys_handle *ssh,
                           u_int8_t type, struct nfnl_callback *cb)
{
        assert(ssh);
        assert(cb);

        if (type >= ssh->cb_count) {
                errno = EINVAL;
                return -1;
        }

The docs say return -1, the code asserts on two arg checks, and
returns an error for the third.

I found this behaviour when calling nfct_callback_register() on a
handle that was opened with the EXPECT subsystem.

It appears to be the prevalent convention in the code, but I'm binding
the nfct APIs into lua, and would like users of the scripting language
to not have abort()ions on misuse of the API.

I'm not super-keen on checking in my binding code all the conditions
in underlying asserts. And there is no public nfct API to determine
what subsystem the handle was opened on, anyhow, so I can't easily
check.

Is it worth supplying patches?

Cheers,
Sam

Re: is assert() an appropriate substitute for return -1?

[Pablo Neira Ayuso]

On 24/03/11 19:23, Sam Roberts wrote:
>  From libnfnetlink.c:
>
>
>   * On success, 0 is returned. On error, -1 is returned and errno is set
>   * appropiately.
>   */
> int nfnl_callback_register(struct nfnl_subsys_handle *ssh,
>                             u_int8_t type, struct nfnl_callback *cb)
> {
>          assert(ssh);
>          assert(cb);
>
>          if (type>= ssh->cb_count) {
>                  errno = EINVAL;
>                  return -1;
>          }
>
> The docs say return -1, the code asserts on two arg checks, and
> returns an error for the third.
>
> I found this behaviour when calling nfct_callback_register() on a
> handle that was opened with the EXPECT subsystem.
>
> It appears to be the prevalent convention in the code, but I'm binding
> the nfct APIs into lua, and would like users of the scripting language
> to not have abort()ions on misuse of the API.
>
> I'm not super-keen on checking in my binding code all the conditions
> in underlying asserts. And there is no public nfct API to determine
> what subsystem the handle was opened on, anyhow, so I can't easily
> check.
>
> Is it worth supplying patches?

I read once in the manpages that you can disable assert() macro with NDEBUG.

Is that enough for you?

Re: is assert() an appropriate substitute for return -1?

[Sam Roberts]

On Mon, Mar 28, 2011 at 5:51 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> Is it worth supplying patches?
>
> I read once in the manpages that you can disable assert() macro with NDEBUG.
>
> Is that enough for you?

No, this only changes the signal number (instead of terminating with
SIGABRT, process will terminate with SISEGV when the null pointers are
used).

The asserts could be changed to #define argassert(x)
if(!x){errno=EINVAL; return -1;} -- mostly the asserts occur before
anything is done that would require cleanup.

Cheers,
Sam