You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/03/06 11:29:00 UTC
Re: svn commit: r36363 - trunk/subversion/libsvn_ra_serf
Geez... making a duplicate to satisfy a const warning? That is *not*
the right approach.
Instead, you can easily see that the handler is not attempting to
modify the header at all. It should have been declared as a const
parameter. *That* is the proper change.
I sent lgo a review of the auth work yesterday, but he may have missed
it. That included adding a lot of const qualifiers.
I'll take care of it. This kind of change is just silly.
-g
On Fri, Mar 6, 2009 at 08:25, Senthil Kumaran S <se...@collab.net> wrote:
> Author: stylesen
> Date: Thu Mar 5 23:25:44 2009
> New Revision: 36363
>
> Log:
> Follow up r36331.
>
> Fix compiler warning of 'passing argument discards qualifiers from pointer
> target type'.
>
> * subversion/libsvn_ra_serf/auth.c
> (handle_auth_header): Pass 'char *' header instead of 'const char *'.
>
> Modified:
> trunk/subversion/libsvn_ra_serf/auth.c
>
> Modified: trunk/subversion/libsvn_ra_serf/auth.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_serf/auth.c?pathrev=36363&r1=36362&r2=36363
> ==============================================================================
> --- trunk/subversion/libsvn_ra_serf/auth.c Thu Mar 5 20:36:58 2009 (r36362)
> +++ trunk/subversion/libsvn_ra_serf/auth.c Thu Mar 5 23:25:44 2009 (r36363)
> @@ -197,7 +197,7 @@ handle_auth_header(void *baton,
> if (strcmp(key, auth_hdr) != 0)
> return 0;
>
> - auth_name = apr_strtok(header, " ", &auth_attr);
> + auth_name = apr_strtok(apr_pstrdup(ab->pool, header), " ", &auth_attr);
> ab->last_prot_name = auth_name;
>
> /* Find the matching authentication handler.
> @@ -235,7 +235,8 @@ handle_auth_header(void *baton,
> proto_found = TRUE;
> ab->prot = prot;
> err = handler(ab->ctx, ab->request, ab->response,
> - header, auth_attr, session->pool);
> + apr_pstrdup(ab->pool, header), auth_attr,
> + session->pool);
> }
> if (err)
> {
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1275824
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1276686
Re: svn commit: r36363 - trunk/subversion/libsvn_ra_serf
Posted by Senthil Kumaran S <se...@collab.net>.
Greg Stein wrote:
> Geez... making a duplicate to satisfy a const warning? That is *not*
> the right approach.
>
> Instead, you can easily see that the handler is not attempting to
> modify the header at all. It should have been declared as a const
> parameter. *That* is the proper change.
Greg, thanks for your comments, I shall see to it that, such things does not
happen in future. Let me get closer in reviewing the changes, than what I ve
did now.
Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1277372
Re: svn commit: r36363 - trunk/subversion/libsvn_ra_serf
Posted by Greg Stein <gs...@gmail.com>.
On Fri, Mar 6, 2009 at 14:16, Lieven Govaerts <lg...@mobsol.be> wrote:
> On Fri, Mar 6, 2009 at 12:29 PM, Greg Stein <gs...@gmail.com> wrote:
>> Geez... making a duplicate to satisfy a const warning? That is *not*
>> the right approach.
>>
>> Instead, you can easily see that the handler is not attempting to
>> modify the header at all. It should have been declared as a const
>> parameter. *That* is the proper change.
>
> Agreed.
>
>>
>> I sent lgo a review of the auth work yesterday, but he may have missed
>> it. That included adding a lot of const qualifiers.
>>
>> I'll take care of it. This kind of change is just silly.
>>
>> -g
>
> I didn't miss it Greg, just didn't have the time yet to work on your
> suggestions. I'll do it this weekend, no need to do it yourself.
Gotcha.
Well... no matter. I've dug into the code and made lots of changes.
const in many places, adding svn_ra_serf__ prefixes, fixing up the
sspi code, revamping some of the parsing code, etc.
If you could do some testing this weekend on my changes, that would be
great. I don't have a good environment for that.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1277477
Re: svn commit: r36363 - trunk/subversion/libsvn_ra_serf
Posted by Lieven Govaerts <lg...@mobsol.be>.
On Fri, Mar 6, 2009 at 12:29 PM, Greg Stein <gs...@gmail.com> wrote:
> Geez... making a duplicate to satisfy a const warning? That is *not*
> the right approach.
>
> Instead, you can easily see that the handler is not attempting to
> modify the header at all. It should have been declared as a const
> parameter. *That* is the proper change.
Agreed.
>
> I sent lgo a review of the auth work yesterday, but he may have missed
> it. That included adding a lot of const qualifiers.
>
> I'll take care of it. This kind of change is just silly.
>
> -g
I didn't miss it Greg, just didn't have the time yet to work on your
suggestions. I'll do it this weekend, no need to do it yourself.
Lieven
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1277243