[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: missing error checking in svn_config__parse_file

From: Ph. Marek <philipp.marek_at_bmlv.gv.at>
Date: 2005-04-04 15:00:29 CEST

On Monday 04 April 2005 13:41, Peter N. Lundblad wrote:
> And if you read that function, you see that itt will modify ch when it
> returns an error. Same holds for parse_section AFAICT.
I saw that.

> If there is a place
> where ch is not set to EOF, please show it or -even better - provide a
> patch to fix that.
Well, my proposed patch for escape sequences :-)

> The problem with your patch (ignoring the leak) is that it makes the code
> very confusing. In many (all I think) places, it sets both err and the
> character to EOF. Then there is a special test at the end of the loop.
Yes.

> > And from my feeling, the test should be for
> >
> > err != SVN_NO_ERROR,
> > not some other secondary variable, which is likely to be forgotten (as it
> > happened me with parsing escape-characters!)
>
> IMO this is a valid point. I don't know why the code is like it is, and
> there is no comment explaining it either. Also, parse_option and
> parse_section are poorly documented.
That's why I overlooked that point.

> Maybe the reason is performance, although I think this extra pointer check
> would hardly make a measurable difference, even in a parsing loop like
> this.
And all functions set err. So it doesn't make sense to set another variable to
a special value too, because testing a char is not faster than testing a
pointer.

> If you think this hsould be fixed, feel free to submit a complete patch
> which we could discuss.
>
> > > OTOH, looking at the code below, it seems that we leak an error if err
> > > got set in the loop and ferror returns true. It needs a deeper look,
> > > though.
> >
> > Ok.
> > Leaving it to you.
>
> Heh...
Well, I don't think I'll find the time this week. So feel free to do that :-)

Regards,

Phil

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Apr 4 15:01:46 2005

This is an archived mail posted to the Subversion Dev mailing list.