Re: 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, [EMAIL PROTECTED] wrote:
 [EMAIL PROTECTED] 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-  [EMAIL PROTECTED]
Samba Team-  http://www.samba.org
Italian Site  -  http://samba.xsec.it

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

[idra]

Author: idra
Date: 2005-06-06 15:19:49 + (Mon, 06 Jun 2005)
New Revision: 7343

WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=revroot=sambarev=7343

Log:
handle url like ldb_tdb does
Modified:
   branches/SAMBA_4_0/source/lib/ldb/ldb_sqlite3/ldb_sqlite3.c


Changeset:
Modified: branches/SAMBA_4_0/source/lib/ldb/ldb_sqlite3/ldb_sqlite3.c
===
--- branches/SAMBA_4_0/source/lib/ldb/ldb_sqlite3/ldb_sqlite3.c 2005-06-06 
15:03:16 UTC (rev 7342)
+++ branches/SAMBA_4_0/source/lib/ldb/ldb_sqlite3/ldb_sqlite3.c 2005-06-06 
15:19:49 UTC (rev 7343)
@@ -903,11 +903,16 @@
 ;
 
 /* 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;
+   }
+
 
 /*
  * See if we'll be creating a new database, or opening an existing one

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

[derrell]

[EMAIL PROTECTED] 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.

Cheers,

Derrell