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 2012/04/25 21:25:59 UTC

Re: svn commit: r1330233 - /subversion/trunk/subversion/libsvn_subr/sqlite.c

On Apr 25, 2012 8:16 AM, <rh...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_subr/sqlite.c Wed Apr 25 12:16:12
2012
> @@ -718,8 +718,23 @@ close_apr(void *data)
>   for (i = 0; i < db->nbr_statements; i++)
>     {
>       if (db->prepared_stmts[i])
> -        err = svn_error_compose_create(
> +        {
> +          if (db->prepared_stmts[i]->needs_reset)
> +            {
> +#ifdef SVN_DEBUG
> +              const char *stmt_text = db->statement_strings[i];
> +              stmt_text = stmt_text; /* Provide value for debugger */
> +
> +              SVN_ERR_MALFUNCTION_NO_RETURN();
> +#else
> +              err = svn_error_compose_create(
> +                            err,
> +                            svn_sqlite__reset(db->prepared_stmts[i]));
> +#endif

I think the reset should be outside the #ifdef. Couldn't the app install an
"ignore" malfunction handler and fall through? If so, we want the statement
reset.

>...

Cheers,
-g

RE: svn commit: r1330233 - /subversion/trunk/subversion/libsvn_subr/sqlite.c

Posted by Bert Huijben <be...@qqmail.nl>.
> 
> On Apr 25, 2012 8:16 AM, <rh...@apache.org> wrote:
> >...
> > +++ subversion/trunk/subversion/libsvn_subr/sqlite.c Wed Apr 25 12:16:12
2012
> > @@ -718,8 +718,23 @@ close_apr(void *data)
> >   for (i = 0; i < db->nbr_statements; i++)
> >     {
> >       if (db->prepared_stmts[i])
> > -        err = svn_error_compose_create(
> > +        {
> > +          if (db->prepared_stmts[i]->needs_reset)
> > +            {
> > +#ifdef SVN_DEBUG
> > +              const char *stmt_text = db->statement_strings[i];
> > +              stmt_text = stmt_text; /* Provide value for debugger */
> > +
> > +              SVN_ERR_MALFUNCTION_NO_RETURN();
> > +#else
> > +              err = svn_error_compose_create(
> > +                            err,
> > +                            svn_sqlite__reset(db->prepared_stmts[i]));
> > +#endif
> I think the reset should be outside the #ifdef. Couldn't the app install
an "ignore" malfunction handler and fall through? If so, we want the
statement reset.

SVN_ERR_MALFUNCTION_NO_RETURN() can't return (as the name implies).

Therefore the compiler will warn that the following code is unreachable.

	Bert


Re: svn commit: r1330233 - /subversion/trunk/subversion/libsvn_subr/sqlite.c

Posted by Daniel Shahaf <da...@elego.de>.
Greg Stein wrote on Wed, Apr 25, 2012 at 15:25:59 -0400:
> On Apr 25, 2012 8:16 AM, <rh...@apache.org> wrote:
> >...
> > +++ subversion/trunk/subversion/libsvn_subr/sqlite.c Wed Apr 25 12:16:12
> 2012
> > @@ -718,8 +718,23 @@ close_apr(void *data)
> >   for (i = 0; i < db->nbr_statements; i++)
> >     {
> >       if (db->prepared_stmts[i])
> > -        err = svn_error_compose_create(
> > +        {
> > +          if (db->prepared_stmts[i]->needs_reset)
> > +            {
> > +#ifdef SVN_DEBUG
> > +              const char *stmt_text = db->statement_strings[i];
> > +              stmt_text = stmt_text; /* Provide value for debugger */
> > +
> > +              SVN_ERR_MALFUNCTION_NO_RETURN();
> > +#else
> > +              err = svn_error_compose_create(
> > +                            err,
> > +                            svn_sqlite__reset(db->prepared_stmts[i]));
> > +#endif
> 
> I think the reset should be outside the #ifdef. Couldn't the app install an
> "ignore" malfunction handler and fall through? If so, we want the statement
> reset.

No.  SVN_ERR_MALFUNCTION_NO_RETURN() doesn't return.  (It abort()s if
the handler returns)