Stefan Beller <sbel...@google.com> writes:

> 'item->dst' has not been assigned if '!rhs' is true. As the caller is allowed 
> to pass in uninitialized
> memory (we don't assume 'item' was zeroed out before calling), this fixes an 
> access to
> uninitialized memory.

Did I miss the other 4 patches that this might depend on it?

> diff --git a/refspec.c b/refspec.c
> index c59a4ccf1e5..ea169dec0d3 100644
> --- a/refspec.c
> +++ b/refspec.c
> @@ -108,7 +108,7 @@ static int parse_refspec(struct refspec_item *item, const 
> char *refspec, int fet
>                * - empty is not allowed.
>                * - otherwise it must be a valid looking ref.
>                */
> -             if (!item->dst) {
> +             if (!rhs) {
>                       if (check_refname_format(item->src, flags))
>                               return 0;
>               } else if (!*item->dst) {

Perhaps a better fisx is to explicitly assign NULL to item->dst when
we see there is no right-hand-side.

Aside from the "uninitialized" issue, the original if/else cascade
around here makes a lot more sense than the updated version.  If we
do not leave item->dst uninitialized, the control (and the readers'
understanding) can flow without having to carry the invariant
"item->dst is set ONLY when rhs != NULL" throughout this codepath,
in order to understand that this if/else cascade is asking: is
pointer NULL?  then do one thing, otherwise is pointee NUL? then do
another thing, otherwise we have a non-empty string so do something
on it.



Reply via email to