Libelektra: Error Concept Improvements Metaissue

[ghost]

Summary of improvement TODOs from PR https://github.com/ElektraInitiative/libelektra/pull/2435

• [x] Write guidelines how to categorize errors.
• [x] Please put memory allocation errors in its own category so that they are easy to find in future.
• [ ] Reduce the size of kdberror.h by refactoring (https://github.com/ElektraInitiative/libelektra/pull/2435#discussion_r269786368)
• [ ] Possibly add a macro ELEKTRA_ADD_SOLUTION which can serve as new method of supplying a solution (https://github.com/ElektraInitiative/libelektra/pull/2435#discussion_r269769574)
• [x] Error code format changes for Warning/Errors (https://github.com/ElektraInitiative/libelektra/pull/2435#discussion_r269767654)
• [ ] Make a concept for the API (https://github.com/ElektraInitiative/libelektra/pull/2435#pullrequestreview-219722372)
• [ ] Errno should be handles by error framework
• [ ] Checker tool to see for unused errors/warnings (https://github.com/ElektraInitiative/libelektra/pull/2435#discussion_r267290712)
• [ ] Optimize the dynamic dispatching in the kdberrors.h to not strcmp for the full code but for the first digits and then for the full string. This reduces the strcmp calls significantly if there are lots of error codes. This performance optimization though should be more relevant if we have much more error codes (currently only 8) (Idea from @kodebach )
• [x] Differentiate the error by the name rather than by the parameter, eg. ELEKTRA_ERROR_RESOURCE(...) instead of ELEKTRA_ERROR(RESOURCE,...). (https://github.com/ElektraInitiative/libelektra/pull/2589#discussion_r273174729)
• [ ] Change the warning key structure to not be limited to 100 entries but instead use arrays (https://github.com/ElektraInitiative/libelektra/pull/2589#discussion_r272829754)
• [x] Incorporate an INFO level (https://github.com/ElektraInitiative/libelektra/issues/2610)
• [ ] Add ElektraError accessor functions to highlevel API (@kodebach idea)

[markus2330]

Thank you for starting collecting the issues!

Please collect all problems/open discussions, even if you maybe will not solve them. E.g. "errno should be handled by error framework".

[ghost]

In #2435 we never talked about errno handling, I think it was another discussion which I do not remember unfortunately.
After going through the discussion I found another task but I think thats it. Do you remember anything else?

[markus2330]

I also did not remember "errno", I found it by skimming the discussion. I expected that you will look through everything properly, so that nothing gets lost.

[markus2330]

Very good that you finally improve our error system!

I just saw #373. It is a very old problem and still unfixed.

[ghost]

So #373 is basically a macro generation where the error type is integrated into the name if I have understand that correctly?

[markus2330]

373 suggested that we introduce something like macro in the specification. Unfortunately, we never completely finished it, so we still have error codes/macros intermixed.

[markus2330]

As discussed in #2610 it would also make sense to have info messages which are not shown by default at all.

[ghost]

This is basically abusing the error message concept to also use for logging. I have never seen anything like that in a project before. There should be another concept or way of logging and saving cache hit messages. I can add it here as idea but it would really be against my intuition to incorporate it into the error concept

[ghost]

373 suggested that we introduce something like macro in the specification. Unfortunately, we never completely finished it, so we still have error codes/macros intermixed.

At the moment we do not have them mixed anymore with the new concept. But I am still unsure what you actually want with #373. A macro entry which is mandatory?

[kodebach]

This is basically abusing the error message concept to also use for logging

I agree with both of you. There is no good concept for info messages in Elektra, but integrating them into the error concept is also not a good solution. The problem IMO is that logging in Elektra is disabled by default and even if Elektra is compiled with -DENABLE_LOGGER=ON, the logging is filtered very heavily (only message from files below src/tools are shown), unless you maintain a separate version of log.c.

IMO we should enable logging to syslog (I think we are missing openlog and closelog) by default for message of level INFO and above, no matter were the originate from.

[mpranj]

I agree, but it would be handy to be able to set the log level by configuration (without recompiling).

Edit: I also think that INFO is too verbose as a default setting.

[markus2330]

Yes, let us discuss this tomorrow.

I think we can easily make the logger more verbose on default. It is disabled by default anyway. And if someone passes -DENABLE_LOGGER=ON he/she obviously wants logs.

But this is actually another problem. The info messages I suggested above are messages that should go to the end user (like warnings+error) and not to the loggers.

[markus2330]

I added two points:

• [ ] Write guidelines how to categorize errors.
• [ ] Please put memory allocation errors in its own category so that they are easy to find in future. (They actually need special handling, the macro would need to do something else.)

The guideline should be done asap as there already seem to be inconsistent categorizations.

[stale[bot]]

I mark this issue stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping the issue by writing a message here or create a new issue with the remainder of this issue.
Thank you for your contributions :sparkling_heart:

[stale[bot]]

I closed this issue now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new issue with the remainder of this issue.
Thank you for your contributions :sparkling_heart: