You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2012/04/27 23:02:39 UTC

Queries on two instances of __bind_int64(revnum) in WC

Can anyone advise on these?

Index: subversion/libsvn_wc/entries.c
===================================================================
--- subversion/libsvn_wc/entries.c    (revision 1331595)
+++ subversion/libsvn_wc/entries.c    (working copy)
@@ -1441,12 +1441,13 @@ insert_node(svn_sqlite__db_t *sdb,
   if (node->repos_relpath)
     {
       SVN_ERR(svn_sqlite__bind_int64(stmt, 5,
                                      node->repos_id));
       SVN_ERR(svn_sqlite__bind_text(stmt, 6,
                                     node->repos_relpath));
+      /* ### Shouldn't this use __bind_revnum?  (If not, say why.) */
       SVN_ERR(svn_sqlite__bind_int64(stmt, 7, node->revision));
     }
 
   if (node->presence == svn_wc__db_status_normal)
     SVN_ERR(svn_sqlite__bind_text(stmt, 8, "normal"));
   else if (node->presence == svn_wc__db_status_not_present)
Index: subversion/libsvn_wc/wc_db.c
===================================================================
--- subversion/libsvn_wc/wc_db.c    (revision 1331595)
+++ subversion/libsvn_wc/wc_db.c    (working copy)
@@ -1082,12 +1082,13 @@ insert_working_node(void *baton,
     }
 
   if (piwb->original_repos_relpath != NULL)
     {
       SVN_ERR(svn_sqlite__bind_int64(stmt, 5, piwb->original_repos_id));
       SVN_ERR(svn_sqlite__bind_text(stmt, 6, piwb->original_repos_relpath));
+      /* ### Shouldn't this use __bind_revnum?  (If not, say why.) */
       SVN_ERR(svn_sqlite__bind_int64(stmt, 7, piwb->original_revnum));
     }
 
   SVN_ERR(svn_sqlite__bind_properties(stmt, 15, piwb->props, scratch_pool));
 
   SVN_ERR(svn_sqlite__insert(NULL, stmt));


- Julian


Re: Queries on two instances of __bind_int64(revnum) in WC

Posted by Julian Foad <ju...@btopenworld.com>.
Daniel Shahaf wrote:

> Julian Foad wrote on Fri, Apr 27, 2012 at 22:04:45 +0100:
>>  One point being the semantics is different: _bind_revnum sets column to 
>> NULL for INVALID_REVNUM.
>> 
>>  Don't know if INVALID_REVNUM ever can hit these two particular code 
>> paths.
> 
> You could check that by throwing an assert() there and running the test
> suite?

Could do; haven't done.

Greg said on IRC 'no reason for them, other than "code written before revnum binding code"', so I just changed them to _revnum in r1331733.

- Julian


>>  > Index: subversion/libsvn_wc/entries.c
>>  > ===================================================================
>>  > +      /* ### Shouldn't this use __bind_revnum?  (If not, say why.) */
>>  >        SVN_ERR(svn_sqlite__bind_int64(stmt, 7, node->revision));
>>  > Index: subversion/libsvn_wc/wc_db.c
>>  > ===================================================================
>>  > +      /* ### Shouldn't this use __bind_revnum?  (If not, say why.) */
>>  >        SVN_ERR(svn_sqlite__bind_int64(stmt, 7, piwb->original_revnum));

Re: Queries on two instances of __bind_int64(revnum) in WC

Posted by Daniel Shahaf <da...@elego.de>.
Julian Foad wrote on Fri, Apr 27, 2012 at 22:04:45 +0100:
> > Can anyone advise on these?
> 
> 
> One point being the semantics is different: _bind_revnum sets column to NULL for INVALID_REVNUM.
> 
> Don't know if INVALID_REVNUM ever can hit these two particular code paths.
> 

You could check that by throwing an assert() there and running the test
suite?

> - Julian
> 
> 
> > Index: subversion/libsvn_wc/entries.c
> > ===================================================================
> > --- subversion/libsvn_wc/entries.c    (revision 1331595)
> > +++ subversion/libsvn_wc/entries.c    (working copy)
> > @@ -1441,12 +1441,13 @@ insert_node(svn_sqlite__db_t *sdb,
> >    if (node->repos_relpath)
> >      {
> >        SVN_ERR(svn_sqlite__bind_int64(stmt, 5,
> >                                       node->repos_id));
> >        SVN_ERR(svn_sqlite__bind_text(stmt, 6,
> >                                      node->repos_relpath));
> > +      /* ### Shouldn't this use __bind_revnum?  (If not, say why.) */
> >        SVN_ERR(svn_sqlite__bind_int64(stmt, 7, node->revision));
> >      }
> >  
> >    if (node->presence == svn_wc__db_status_normal)
> >      SVN_ERR(svn_sqlite__bind_text(stmt, 8, "normal"));
> >    else if (node->presence == svn_wc__db_status_not_present)
> > Index: subversion/libsvn_wc/wc_db.c
> > ===================================================================
> > --- subversion/libsvn_wc/wc_db.c    (revision 1331595)
> > +++ subversion/libsvn_wc/wc_db.c    (working copy)
> > @@ -1082,12 +1082,13 @@ insert_working_node(void *baton,
> >      }
> >  
> >    if (piwb->original_repos_relpath != NULL)
> >      {
> >        SVN_ERR(svn_sqlite__bind_int64(stmt, 5, piwb->original_repos_id));
> >        SVN_ERR(svn_sqlite__bind_text(stmt, 6, piwb->original_repos_relpath));
> > +      /* ### Shouldn't this use __bind_revnum?  (If not, say why.) */
> >        SVN_ERR(svn_sqlite__bind_int64(stmt, 7, piwb->original_revnum));
> >      }
> >  
> >    SVN_ERR(svn_sqlite__bind_properties(stmt, 15, piwb->props, scratch_pool));
> >  
> >    SVN_ERR(svn_sqlite__insert(NULL, stmt));
> > 
> > 
> > - Julian
> >

Re: Queries on two instances of __bind_int64(revnum) in WC

Posted by Julian Foad <ju...@btopenworld.com>.
> Can anyone advise on these?


One point being the semantics is different: _bind_revnum sets column to NULL for INVALID_REVNUM.

Don't know if INVALID_REVNUM ever can hit these two particular code paths.

- Julian


> Index: subversion/libsvn_wc/entries.c
> ===================================================================
> --- subversion/libsvn_wc/entries.c    (revision 1331595)
> +++ subversion/libsvn_wc/entries.c    (working copy)
> @@ -1441,12 +1441,13 @@ insert_node(svn_sqlite__db_t *sdb,
>    if (node->repos_relpath)
>      {
>        SVN_ERR(svn_sqlite__bind_int64(stmt, 5,
>                                       node->repos_id));
>        SVN_ERR(svn_sqlite__bind_text(stmt, 6,
>                                      node->repos_relpath));
> +      /* ### Shouldn't this use __bind_revnum?  (If not, say why.) */
>        SVN_ERR(svn_sqlite__bind_int64(stmt, 7, node->revision));
>      }
>  
>    if (node->presence == svn_wc__db_status_normal)
>      SVN_ERR(svn_sqlite__bind_text(stmt, 8, "normal"));
>    else if (node->presence == svn_wc__db_status_not_present)
> Index: subversion/libsvn_wc/wc_db.c
> ===================================================================
> --- subversion/libsvn_wc/wc_db.c    (revision 1331595)
> +++ subversion/libsvn_wc/wc_db.c    (working copy)
> @@ -1082,12 +1082,13 @@ insert_working_node(void *baton,
>      }
>  
>    if (piwb->original_repos_relpath != NULL)
>      {
>        SVN_ERR(svn_sqlite__bind_int64(stmt, 5, piwb->original_repos_id));
>        SVN_ERR(svn_sqlite__bind_text(stmt, 6, piwb->original_repos_relpath));
> +      /* ### Shouldn't this use __bind_revnum?  (If not, say why.) */
>        SVN_ERR(svn_sqlite__bind_int64(stmt, 7, piwb->original_revnum));
>      }
>  
>    SVN_ERR(svn_sqlite__bind_properties(stmt, 15, piwb->props, scratch_pool));
>  
>    SVN_ERR(svn_sqlite__insert(NULL, stmt));
> 
> 
> - Julian
>