[no patch]: source3/utils/regedit_valuelist.c

Jeremy Allison jra at samba.org
Thu Feb 13 18:51:59 MST 2014


On Thu, Feb 13, 2014 at 03:18:50PM -0600, Christopher R. Hertel wrote:
> So... looking at a Coverity defect, I see an obvious but only partial fix:
> 
> --- a/source3/utils/regedit_valuelist.c
> +++ b/source3/utils/regedit_valuelist.c
> @@ -169,7 +169,7 @@ static bool string_is_printable(const char *s)
> 
>  static WERROR append_data_summary(struct value_item *vitem)
>  {
> -       char *tmp;
> +       char *tmp = NULL;
> 
>  /* This is adapted from print_registry_value() in net_registry_util.c */
> 
> 
> Initializing the variable <tmp> to NULL is needed, because there are two
> instances in which we can break out of the switch() statement without
> setting any value to <tmp>.
> 
>                 if (!pull_reg_sz(vitem, &vitem->data, &s)) {
>                         break;
>                 }
> 
> and
> 
>                 if (!pull_reg_multi_sz(vitem, &vitem->data, &a)) {
>                         break;
>                 }
> 
> 
> However, at the end of the function, we have:
> 
>         if (tmp == NULL) {
>                 return WERR_NOMEM;
>         }
> 
> So the problem is this:  What error value *should* be returned if either
> pull_reg_sz() or pull_reg_multi_sz() fail?  WERR_NOMEM seems like the wrong
> result in those cases.

They can fail if the underlying NDR synchronization fails
(which means an underlying corrupted registry) or if they
actually run out of memory. So IMHO WERR_NOMEM is OK to
return in these cases.

Jeremy.


More information about the samba-technical mailing list