svn commit: samba r7343 - in branches/SAMBA_4_0/source/lib/ldb/ldb_sqlite3: .

[Simo Sorce]

On Mon, 2005-06-06 at 13:04 -0400, derrell at samba.org wrote:
> idra at samba.org writes:
> 
> >          /* Skip protocol indicator of url  */
> > -        if ((p = strchr(url, ':')) == NULL) {
> > -                return SQLITE_MISUSE;
> > -        } else {
> > -                ++p;
> > -        }
> > +	if (strchr(url, ':')) {
> > +		if (strncmp(url, "sqlite://", 9) != 0) {
> > +			errno = EINVAL;
> > +			return SQLITE_MISUSE;
> > +		}
> > +		p = url + 9;
> > +	} else {
> > +		p = url;
> > +	}
> > +
> 
> Simo, I believe this is a waste.  We can't (or at least, shouldn't be able to)
> get here (into ldb_sqlite3) without the url having a proper protocol
> indicator.  Your new method scans the string twice (once in strchr() and again
> in strncmp(), unnecessarily I think.  If it is possible to get here with a
> protocol indicator other than the intended one, the higher-level (calling)
> code should be fixed to ensure that can not happen.  Then, the code I had
> intentionally replaced the tdb code with (pre- this change) is more efficient.

the connect function happen once per usage, a strcmp is very cheap and
we do not have any performance problem to care about in this point of
the code.
I've changed the code, because your code does not implement correctly
our url syntax.

The url must be of this form:

protocol://<path>

Your code admits only absolute paths while the code I've put in make you
possible to use a relative path:

sqlite://test.ldb
and
sqlite:///test.ldb
are different

the first is relative to the current directory, the last is absolute,
your code actually works only because the system reduces the '/' in the
head so that the path used is /test.ldb any number of '/' you put
upfront.

Please either use the code I've put in or make your code respect our
syntax. 

Simo.

-- 
Simo Sorce    -  idra at samba.org
Samba Team    -  http://www.samba.org
Italian Site  -  http://samba.xsec.it