[sr-dev] PKG/SHM allocation failure log macros

[Henning Westerholt]

Am Freitag, 28. Dezember 2018, 16:21:59 CET schrieb Daniel-Constantin Mierla:
> I noticed many commits replacing the log messages in case of allocation
> failure with some macros. That is good, bringing consistency, but I
> think that we should offer couple of them. The current one is rather dry
> (meaning that it offers very few context details), which matches most of
> the existing log messages used in such cases.

Hi Daniel,

yes - I noticed that these error message were really inconsistent, sometimes 
even hard to understand for normal users (like "running out of pkg").

This was my motivation to do the conversion. Additionally it is this way much 
easier to analyze the log files for this particular errors.
 
> But there are also other log messages for such cases which give more
> details, like for what the allocation fails, some also giving the
> requested size of allocation.

In the core this was done inconsistently (like some error msg were more 
detailed than others, sometimes in the same function). Sometimes the error 
message was wrong because of some changes to the code. Therefore I removed it 
in some cases.

For modules that allocate bigger memory blocks (htable, lcr etc..) I can see 
why it make sense to output more information.

As the core is using really small memory blocks, I don't think information 
about the size is really useful here. The usual response to this problem would 
be to just increase the PKG memory pool.

The location were the allocation error happens can be usually also spot from 
the logging message (the function name).
 
> So besides the current two macros (one for shm and one for pkg), we
> should add few more. Like:
> 
> #define PKG_MEM_ERROR_MSG(m) LM_ERR("could not allocate private memory
> from pkg pool - %s\n", m);
> 
> So one can do:
> 
> PKG_MEM_ERROR_MSG("needed for htable struct");
> 
> And one to include also the size:
> 
> #define PKG_MEM_ERROR_SZ(s, m) LM_ERR("could not allocate private memory
> from pkg pool - size: %u - %s\n", (unsigned int)s, m);

I will add macros for this two additional cases, probably this evening.
 
> No need to revert what was done, but I think for the future we would
> preserve better information for troubleshooting in some cases, instead
> of replacing those messages that now have more details with the bare
> error log message.

See my comment above, in which conditions do you think we should preserve the 
size and additional error message during the conversation?

Best regards,

Henning



-- 
Henning Westerholt - https://skalatan.de/blog/
Kamailio services - https://skalatan.de/services
Kamailio security assessment - https://skalatan.de/de/assessment