Another one liner. If no rules were passed to sdap_sudo_rules_refresh_send (if rules[0] was NULL) and no error occurred, then variable ret was used uninitialized. This is not very likely to happen, but it is safer to have it initialized.
Maybe we should consider adding initialization of all "ret" variables to our coding guidelines to avoid these "unlikely but possible" errors in new code.
Patch attached.
Michal
On Wed 26 Sep 2012 01:20:25 PM EDT, Michal Židek wrote:
Another one liner. If no rules were passed to sdap_sudo_rules_refresh_send (if rules[0] was NULL) and no error occurred, then variable ret was used uninitialized. This is not very likely to happen, but it is safer to have it initialized.
Maybe we should consider adding initialization of all "ret" variables to our coding guidelines to avoid these "unlikely but possible" errors in new code.
Nack. We usually leave ret intentionally uninitialized because we always want to set it explicitly under normal operation. This is because an uninitialized use is detectable by the compiler or static analysis tools. If we pre-initialized it, we could just end up with unexpected behavior (falling into the default case).
Please make all assignments to 'ret' be intentional rather than default.
On 09/26/2012 07:30 PM, Stephen Gallagher wrote:
On Wed 26 Sep 2012 01:20:25 PM EDT, Michal Židek wrote:
Another one liner. If no rules were passed to sdap_sudo_rules_refresh_send (if rules[0] was NULL) and no error occurred, then variable ret was used uninitialized. This is not very likely to happen, but it is safer to have it initialized.
Maybe we should consider adding initialization of all "ret" variables to our coding guidelines to avoid these "unlikely but possible" errors in new code.
Nack. We usually leave ret intentionally uninitialized because we always want to set it explicitly under normal operation. This is because an uninitialized use is detectable by the compiler or static analysis tools. If we pre-initialized it, we could just end up with unexpected behavior (falling into the default case).
Please make all assignments to 'ret' be intentional rather than default.
Hmm.. I forgot to send new version of this patch. The ret variable is now set to EOK when program reaches the end without any error, so it is not used with garbage value in the clean-up.
Thanks Michal
On Wed, Oct 03, 2012 at 10:36:35AM +0200, Michal Židek wrote:
On 09/26/2012 07:30 PM, Stephen Gallagher wrote:
On Wed 26 Sep 2012 01:20:25 PM EDT, Michal Židek wrote:
Another one liner. If no rules were passed to sdap_sudo_rules_refresh_send (if rules[0] was NULL) and no error occurred, then variable ret was used uninitialized. This is not very likely to happen, but it is safer to have it initialized.
Maybe we should consider adding initialization of all "ret" variables to our coding guidelines to avoid these "unlikely but possible" errors in new code.
Nack. We usually leave ret intentionally uninitialized because we always want to set it explicitly under normal operation. This is because an uninitialized use is detectable by the compiler or static analysis tools. If we pre-initialized it, we could just end up with unexpected behavior (falling into the default case).
Please make all assignments to 'ret' be intentional rather than default.
Hmm.. I forgot to send new version of this patch. The ret variable is now set to EOK when program reaches the end without any error, so it is not used with garbage value in the clean-up.
Thanks Michal
Ack
On Wed, Oct 03, 2012 at 04:52:47PM +0200, Jakub Hrozek wrote:
On Wed, Oct 03, 2012 at 10:36:35AM +0200, Michal Židek wrote:
On 09/26/2012 07:30 PM, Stephen Gallagher wrote:
On Wed 26 Sep 2012 01:20:25 PM EDT, Michal Židek wrote:
Another one liner. If no rules were passed to sdap_sudo_rules_refresh_send (if rules[0] was NULL) and no error occurred, then variable ret was used uninitialized. This is not very likely to happen, but it is safer to have it initialized.
Maybe we should consider adding initialization of all "ret" variables to our coding guidelines to avoid these "unlikely but possible" errors in new code.
Nack. We usually leave ret intentionally uninitialized because we always want to set it explicitly under normal operation. This is because an uninitialized use is detectable by the compiler or static analysis tools. If we pre-initialized it, we could just end up with unexpected behavior (falling into the default case).
Please make all assignments to 'ret' be intentional rather than default.
Hmm.. I forgot to send new version of this patch. The ret variable is now set to EOK when program reaches the end without any error, so it is not used with garbage value in the clean-up.
Thanks Michal
Ack
Pushed to master.
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Michal Židek -
Stephen Gallagher