Re: svn commit: r26052 - trunk/subversion/libsvn_client

[David James <ja...@cs.toronto.edu>]

On 8/13/07, hwright@tigris.org <hw...@tigris.org> wrote:
> Author: hwright
> Date: Mon Aug 13 08:13:16 2007
> New Revision: 26052
>
> Log:
> Fix a segfault for consumers of svn_client_copy4() and svn_client_move5(), who
> don't request commit info.
>
> Suggested by: djames
>
> * subversion/libsvn_client/copy.c
>   (svn_client_copy4, svn_client_move5): Ensure that commit_info_p is not NULL
>   before attempting to set it.
>
>
> Modified:
>    trunk/subversion/libsvn_client/copy.c
>
> Modified: trunk/subversion/libsvn_client/copy.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/copy.c?pathrev=26052&r1=26051&r2=26052
> ==============================================================================
> --- trunk/subversion/libsvn_client/copy.c       (original)
> +++ trunk/subversion/libsvn_client/copy.c       Mon Aug 13 08:13:16 2007
> @@ -1942,10 +1942,11 @@
>                         subpool);
>      }
>
> -  if (commit_info)
> -    *commit_info_p = svn_commit_info_dup(commit_info, pool);
> -  else
> -    *commit_info_p = commit_info;
> +  if (commit_info_p != NULL)
> +    if (commit_info)
> +      *commit_info_p = svn_commit_info_dup(commit_info, pool);
> +    else
> +      *commit_info_p = commit_info;

Thanks for the fix, Hyrum. I believe this fix is correct, but I find
it hard to parse because of the dangling else. Could you add some
braces around the outer if statement here to make the code a bit more
clear?

>    svn_pool_destroy(subpool);
>    return err;
> @@ -2094,11 +2095,12 @@
>                         ctx,
>                         subpool);
>      }
> -
> -  if (commit_info)
> -    *commit_info_p = svn_commit_info_dup(commit_info, pool);
> -  else
> -    *commit_info_p = commit_info;
> +
> +  if (commit_info_p != NULL)
> +    if (commit_info)
> +      *commit_info_p = svn_commit_info_dup(commit_info, pool);
> +    else
> +      *commit_info_p = commit_info;

Same here.

Cheers,

David

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r26052 - trunk/subversion/libsvn_client

["Hyrum K. Wright" <hy...@mail.utexas.edu>]

David James wrote:
> On 8/13/07, hwright@tigris.org <hw...@tigris.org> wrote:
...
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/copy.c?pathrev=26052&r1=26051&r2=26052
>> ==============================================================================
>> --- trunk/subversion/libsvn_client/copy.c       (original)
>> +++ trunk/subversion/libsvn_client/copy.c       Mon Aug 13 08:13:16 2007
>> @@ -1942,10 +1942,11 @@
>>                         subpool);
>>      }
>>
>> -  if (commit_info)
>> -    *commit_info_p = svn_commit_info_dup(commit_info, pool);
>> -  else
>> -    *commit_info_p = commit_info;
>> +  if (commit_info_p != NULL)
>> +    if (commit_info)
>> +      *commit_info_p = svn_commit_info_dup(commit_info, pool);
>> +    else
>> +      *commit_info_p = commit_info;
> 
> Thanks for the fix, Hyrum. I believe this fix is correct, but I find
> it hard to parse because of the dangling else. Could you add some
> braces around the outer if statement here to make the code a bit more
> clear?

Mike beat me to it r26056.

-Hyrum