Sorry for the late response,
On Wed, Sep 23, 2020 at 03:58:31PM +0200, Neels Hofmeyr wrote:
On Wed, Sep 23, 2020 at 05:40:39PM +0700, Vadim Yanitskiy wrote:
struct cmd_node foo_node = { .usrattr = (1 << FOO_VTY_ATTR_RESTART_FULL), // (!) };
this way we could no longer see that a newly added command failed to set the correct user attribute. i.e. I add a new DEFUN, I fail to remember setting attributes, and thus it shows some attribute which is wrong. If each command gets explicit attributes, errors like that are unlikely: If I add a DEFUN and forget to set the right attribute, there will be no attribute, and a user reading the vty reference will ask us to fix, without the need to try and fail first.
we could have a checker script that issues warnings, similar to the value_string checker? We could indeed also deprecate DEFUN (make it print a warning for now) while things are migrated to those with attributes...
libosmocore defined attrs would be useful.
ACK.
I know that the VTY code disregards osmo_ completely, but I'd still favor prepending OSMO_* to new enum values.
I'm -as usual - in favor of consistency. So if the old code has no prefix, any additions should also not have prefixes.
btw, each and every osmo_fsm caller and now also each and every vty attribute defining code defines a left-shift macro like
#define S(x) (1 << (x))
it's a bit silly to repeat this definition all over the place. Added to libosmocore, it would have to be called OSMO_S()... makes everything longer. maybe nevermind, just thinking aloud. }
Yes, that is a known issue..
dexter wrote:
Maybe we could have some standard letters defined in libosmocore for standard situations: F = Full restart required I = Applied Immediately
ACK. We can also agree that generic (pre-defined) attributes use upper case letters, while the application specific ones use lower case?
I like that approach
Then again, 'F' should mean the same thing in various osmo-* programs?
yes.
But consider, having attributes of upper case and lower case of the same letter could be somewhat confusing to users.
Well, given that in iptables, lowercase are matches and uppercase are targets, I guess it's clear I don't share such a concern. But maybe it is confusing to Windows users who are taught for decades that there is no distinction between upper and lower case? I'm willing to accept that there is a concern.
libosmocore assigns letters, applications assign numbers and special characters??
I would go for the other way around, as I would expect libosmocore to define fewer 'common' attributes, while applications may have more different flags. So as there are fewer numbers than lstters, I'd go for libosmocore=numbers.
Please note that in the end it's not just libosmcoore, but also libosmogb, libosmo-abis, libosmo-sccp, ...
Regards, Harald