On 14 Nov 2013, at 14:30, John Dennis <jdennis@redhat.com> wrote:
On 11/14/2013 08:29 AM, Arran Cudbard-Bell wrote:
On 14 Nov 2013, at 02:48, John Dennis <jdennis@redhat.com> wrote:
On the ppc64 and s390 architectures radiusd fails to start emitting these errors:
Error: /etc/raddb/mods-enabled/echo[34]: Cannot read output pairs if wait = no Error: /etc/raddb/mods-enabled/echo[34]: Instantiation failed for module "echo"
The problem is due to using the wrong type in the module config struct passed to the config file parser. The type was declared as bool but conffile.c treats PW_TYPE_BOOLEAN as an int. This causes an if test to fail because the bool value is not true when it should be. This is in rlm_exec.c. I found 1 other module which had bool's declared for PW_TYPE_BOOLEAN config items, rlm_pap.c.
Out of interest did the compiler emit warnings about the size mismatch?
No compiler warnings. I also did a Coverity scan of the source tree and although a number of warnings were reported this issue was not detected. However I'm not surprised, the way the code works with pointer offsets to void * (untyped) targets it would be remarkable if a tool could deduce incorrect type declarations.
Quite. Ok i've pushed a proper fix for this. Converting those fields to int would of fixed the immediate problem, but other issues would of arisen later, because of the size mismatch. Some modules already used the bool type in their instance data structs, and the config parser would of overwritten the subsequent 3 bytes of the struct with zeros. Using ints to represent boolean values makes the code harder to follow, especially when they are used as function return types, as it's unclear whether the function will use the standard 0, -1 -n return codes or 1, 0 return codes from reading the signature. Once you start using them in one place, not using them in others is confusing, and would eventually end up making the code more complex. Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team