You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2011/09/15 00:06:49 UTC

Re: svn commit: r1170836 - in /subversion/trunk/subversion: include/ libsvn_client/ libsvn_delta/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_repos/ libsvn_wc/ svnrdump/ svnsync/ tests/libsvn_delta/

hwright@apache.org wrote on Wed, Sep 14, 2011 at 20:28:39 -0000:
> @@ -240,12 +243,19 @@ ev2_change_dir_prop(void *dir_baton,
>                      apr_pool_t *scratch_pool)
>  {
>    struct ev2_dir_baton *db = dir_baton;
> -  struct prop_args *p_args = apr_palloc(db->eb->edit_pool, sizeof(*p_args));
>  
> -  p_args->name = apr_pstrdup(db->eb->edit_pool, name);
> -  p_args->value = value ? svn_string_dup(value, db->eb->edit_pool) : NULL;
> +  if (value)
> +    {
> +      struct prop_args *p_args = apr_palloc(db->eb->edit_pool, sizeof(*p_args));
> +
> +      p_args->name = apr_pstrdup(db->eb->edit_pool, name);
> +      p_args->value = value ? svn_string_dup(value, db->eb->edit_pool) : NULL;

The NULL will never be evaluated.  (also in ev2_change_file_prop())

The rest looks good.

Re: svn commit: r1170836 - in /subversion/trunk/subversion: include/ libsvn_client/ libsvn_delta/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_repos/ libsvn_wc/ svnrdump/ svnsync/ tests/libsvn_delta/

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Hyrum K Wright wrote on Wed, Sep 14, 2011 at 20:30:03 -0500:
> On Wed, Sep 14, 2011 at 5:06 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > The rest looks good.
> 
> Thanks.  In spite of my prickly attitude a couple of days ago, I do
> appreciate the review.  :)

Thanks.  That's the spirit it's intended in (even if I may sound
otherwise...)

Re: svn commit: r1170836 - in /subversion/trunk/subversion: include/ libsvn_client/ libsvn_delta/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_repos/ libsvn_wc/ svnrdump/ svnsync/ tests/libsvn_delta/

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Hyrum K Wright wrote on Wed, Sep 14, 2011 at 20:30:03 -0500:
> On Wed, Sep 14, 2011 at 5:06 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > The rest looks good.
> 
> Thanks.  In spite of my prickly attitude a couple of days ago, I do
> appreciate the review.  :)

Thanks.  That's the spirit it's intended in (even if I may sound
otherwise...)

Re: svn commit: r1170836 - in /subversion/trunk/subversion: include/ libsvn_client/ libsvn_delta/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_repos/ libsvn_wc/ svnrdump/ svnsync/ tests/libsvn_delta/

Posted by Hyrum K Wright <hy...@wandisco.com>.
On Wed, Sep 14, 2011 at 5:06 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> hwright@apache.org wrote on Wed, Sep 14, 2011 at 20:28:39 -0000:
>> @@ -240,12 +243,19 @@ ev2_change_dir_prop(void *dir_baton,
>>                      apr_pool_t *scratch_pool)
>>  {
>>    struct ev2_dir_baton *db = dir_baton;
>> -  struct prop_args *p_args = apr_palloc(db->eb->edit_pool, sizeof(*p_args));
>>
>> -  p_args->name = apr_pstrdup(db->eb->edit_pool, name);
>> -  p_args->value = value ? svn_string_dup(value, db->eb->edit_pool) : NULL;
>> +  if (value)
>> +    {
>> +      struct prop_args *p_args = apr_palloc(db->eb->edit_pool, sizeof(*p_args));
>> +
>> +      p_args->name = apr_pstrdup(db->eb->edit_pool, name);
>> +      p_args->value = value ? svn_string_dup(value, db->eb->edit_pool) : NULL;
>
> The NULL will never be evaluated.  (also in ev2_change_file_prop())

Good catch, fixed in r1170915.

> The rest looks good.

Thanks.  In spite of my prickly attitude a couple of days ago, I do
appreciate the review.  :)

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/

Re: svn commit: r1170836 - in /subversion/trunk/subversion: include/ libsvn_client/ libsvn_delta/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_repos/ libsvn_wc/ svnrdump/ svnsync/ tests/libsvn_delta/

Posted by Hyrum K Wright <hy...@wandisco.com>.
On Wed, Sep 14, 2011 at 5:06 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> hwright@apache.org wrote on Wed, Sep 14, 2011 at 20:28:39 -0000:
>> @@ -240,12 +243,19 @@ ev2_change_dir_prop(void *dir_baton,
>>                      apr_pool_t *scratch_pool)
>>  {
>>    struct ev2_dir_baton *db = dir_baton;
>> -  struct prop_args *p_args = apr_palloc(db->eb->edit_pool, sizeof(*p_args));
>>
>> -  p_args->name = apr_pstrdup(db->eb->edit_pool, name);
>> -  p_args->value = value ? svn_string_dup(value, db->eb->edit_pool) : NULL;
>> +  if (value)
>> +    {
>> +      struct prop_args *p_args = apr_palloc(db->eb->edit_pool, sizeof(*p_args));
>> +
>> +      p_args->name = apr_pstrdup(db->eb->edit_pool, name);
>> +      p_args->value = value ? svn_string_dup(value, db->eb->edit_pool) : NULL;
>
> The NULL will never be evaluated.  (also in ev2_change_file_prop())

Good catch, fixed in r1170915.

> The rest looks good.

Thanks.  In spite of my prickly attitude a couple of days ago, I do
appreciate the review.  :)

-Hyrum



-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/