RPM Community Forums

Mailing List Message of <rpm-devel>

Re: RPM: rpm/rpmdb/ db3.c dbconfig.c rpmdb.c rpmdb.h

From: Alexey Tourbin <at@altlinux.ru>
Date: Mon 18 Aug 2008 - 21:29:36 CEST
Message-ID: <20080818192936.GY618@altlinux.org>
On Mon, Aug 18, 2008 at 12:20:36PM -0400, Jeff Johnson wrote:
> >  @@ -2878,6 +2878,7 @@
> >
> >   	if (db->db_tags != NULL)
> >   	for (dbix = 0; dbix < db->db_ndbi; dbix++) {
> >  +	    dbiIndex dbi;

(Actually one of my previous changes moved these variables from the
outer scope to the inner loop scope.)

Note that dbi corresponds to dbix here, and this is the right scope
for dbi.  There's a temptation to reuse this dbi in another loop,
but that would be a different "dbi" for another purpose.

So, sometimes I ask, like, "what this variable is for?"  One approach
is to use more distinctive names, e.g. s/dbi_for_dbix_index_update/ (and
another would be dbi_to_fetch_max_header_instance_number).  Possibly a
better approach is still to use short names but within the inner scope.
The very scope can tell then what the variable is supposed to mean.

> >   	    DBC * dbcursor = NULL;
> >   	    DBT k = DBT_INIT;
> >   	    DBT v = DBT_INIT;

Now note that k and v are also connected to "dbi" index (k has tag value
for dbi index and v is a set of header instances).  As soon as "dbi" goes
away, k and v have garbage.  They'd better go away together.

> >  @@ -2887,7 +2888,6 @@
> >   	    rpmTag rpmtag = dbiTag->tag;
> >   	    const char * dbiBN = (dbiTag->str != NULL
> >   		? dbiTag->str : tagName(rpmtag));
> >  -	    dbiIndex dbi;
> >   	    rpmuint8_t * bin = NULL;
> >   	    int i;
> >
> >  @@ -2939,7 +2939,6 @@
> >   		if (!xx)
> >   		    continue;
> >   		/*@switchbreak@*/ break;
> >  -
> >   	    }
> >   	
> >   	  dbi = dbiOpen(db, he->tag, 0);

Note that dbiOpen happens to be at the very same scope/indentation as
dbi was first declared.  So is dbiClose, right before the bracket that
closes the scope of the loop.  The benefit at least that you don't have
to set dbi to NULL so that it does not dangle throughout the rest of the
code.

> Be very careful re-scoping dbiIndex here as well. Or at least watch  
> for subtle flaws,
> valgrind should catch any flaws iirc.

I hope I am careful (well, I did not run valgrind, I just asked myself
questions, like, "what this variable is for").


  • application/pgp-signature attachment: stored
Received on Mon Aug 18 21:30:09 2008
Driven by Jeff Johnson and the RPM project team.
Hosted by OpenPKG and Ralf S. Engelschall.
Powered by FreeBSD and OpenPKG.