You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2006/01/30 13:56:46 UTC
Re: [PATCH] Fixing possible segmentation fault
Julian Foad <ju...@btopenworld.com> writes:
> Daniel Berlin wrote:
> > Alexander Thomas wrote:
> >>svn_client_checkout2() fails with segmentation fault, if called with a
> >>revision argument as NULL. IMHO revision should be checked for NULL
> >>before using it further.
> > This is wrong in one of two ways.
> > Either
> > 1. We instead should just assert that revision != NULL, like we do for URL.
> > Nobody should be passing in a NULL revision to this function.
>
> +1 on this. The general rule is that you must not pass a null pointer
> to any API unless its documentation says you may, and in this case it
> does not say so.
I agree with Julian and Daniel here, but also, what was the context?
Did you find someone passing NULL for that parameter? (I.e., is there
some larger issue that needs correcting?)
Thanks,
-Karl
--
www.collab.net <> CollabNet | Distributed Development On Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Fixing possible segmentation fault
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Hi, Alexander,
On Thu, 2 Feb 2006, Alexander Thomas wrote:
> On Mon, 2006-01-30 at 07:56 -0600, kfogel@collab.net wrote:
> > I agree with Julian and Daniel here, but also, what was the context?
> > Did you find someone passing NULL for that parameter? (I.e., is there
> > some larger issue that needs correcting?)
> >
> I don't know about anyone passing NULL, I am culprit here.
>
> I passed NULL because I don't have a revision to pass. I know about the
> svn_opt_revision_unspecified and how to pass it, but was bit lazy and
> never though NULL could have break svn_client_checkout2().
>
You should assume that, unless the API docs say NULL is acceptable.
> I strongly feel that asserting revision (also peg revision) parameter
> will be good idea, because in the wild people can do all sorts of nasty
> things to our API.
>
And if they do, they'll get a crash and learn what not to do:-)
The problem is that if we start doing this, we will have to assert that
every parameter is valid. And it doesn't help much, because if the
parameter is NULL, you will get a crash anyway.
Best,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Fixing possible segmentation fault
Posted by Alexander Thomas <al...@collab.net>.
On Mon, 2006-01-30 at 07:56 -0600, kfogel@collab.net wrote:
> Julian Foad <ju...@btopenworld.com> writes:
> > Daniel Berlin wrote:
> > > Alexander Thomas wrote:
> > >>svn_client_checkout2() fails with segmentation fault, if called with a
> > >>revision argument as NULL. IMHO revision should be checked for NULL
> > >>before using it further.
> > > This is wrong in one of two ways.
> > > Either
> > > 1. We instead should just assert that revision != NULL, like we do for URL.
> > > Nobody should be passing in a NULL revision to this function.
> >
> > +1 on this. The general rule is that you must not pass a null pointer
> > to any API unless its documentation says you may, and in this case it
> > does not say so.
>
> I agree with Julian and Daniel here, but also, what was the context?
> Did you find someone passing NULL for that parameter? (I.e., is there
> some larger issue that needs correcting?)
>
I don't know about anyone passing NULL, I am culprit here.
I passed NULL because I don't have a revision to pass. I know about the
svn_opt_revision_unspecified and how to pass it, but was bit lazy and
never though NULL could have break svn_client_checkout2().
I strongly feel that asserting revision (also peg revision) parameter
will be good idea, because in the wild people can do all sorts of nasty
things to our API.
Please comment
-AT
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org