Re: [PATCH nft 1/2] utils: add memory_allocation_check() helper

[Thomas Haller <thaller@redhat.com>]

On Wed, 2023-11-15 at 09:52 +0100, Florian Westphal wrote:
> Thomas Haller <thaller@redhat.com> wrote:
> > static inline void *__memory_allocation_check(const char *file,
> > unsigned line, const void *ptr) {
> >     if (!ptr)
> >         __memory_allocation_error(file, line);
> >     return (void*) ptr;
> > }
> > 
> > #define memory_allocation_check(cmd) \
> >    ((typeof(cmd) __memory_allocation_check(__FILE__, __LINE__,
> > (cmd))
> > 
> > Doesn't seem to make a difference either way.
> 
> We seem to be moving in circles.
> 
> I suspect your agenda is to avoid repeating the existing
> 
> x = alloc()
> if (!x)
>   barf()
> 
> pattern when adding userhandle support?

exactly.

> 
> If so I think its best to just add a specific ubuf alloc wrapper that
> can't fail (i.e. like the 'xmalloc' wrappers).

I don't understand.

_nftnl_udata_buf_alloc() *is* that specific wrapper.

Maybe the name is bad... and such wrappers should have a "nft_x_"
prefix (nft_x_nftnl_udata_buf_alloc()).

> 
> Like Pablo said, I don't see any added value in providing FILE/LINE
> errors on stderr here.  It could be as simple as exit().
> 

I don't understand.

The two patches don't change anything about that. There is no change in
behavior -- aside introducing a convenience wrapper for the repeated
ENOMEM check.

If you want to change memory_allocation_error() to not use FILE/LINE or
call exit(), then that is a separate discussion (I'd like to avoid).


Thomas