PATCH -- remove duplicate paragraph from texinfo

[Peter Breitenlohner]

Hi,

the attached patch removes a duplicate paragraph from find.texi and fixes a
formatting typo. I hope that can also be applied to the 4.4 branch.

Regards
Peter Breitenlohner <peb@...>
From 41088ce364bd1154b918c838947145320f14377d Mon Sep 17 00:00:00 2001
From: Peter Breitenlohner <peb@...>
Date: Mon, 3 Aug 2009 11:16:29 +0200
Subject: [PATCH] Remove duplicate paragraph in texinfo

Signed-off-by: Peter Breitenlohner <peb@...>
---
 ChangeLog     |    4 ++++
 doc/find.texi |   12 +-----------
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9c3290a..fd14c44 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
[phone]  Peter Breitenlohner  <peb@...>
+
+ * doc/find.texi: Remove duplicate paragraph and fix a typo.
+
 2009-07-18  James Youngman  <jay@...>
 
  Fix up some problems introduced in a manual conflict resolution.
diff --git a/doc/find.texi b/doc/find.texi
index 24ee8a8..5a2a121 100644
--- a/doc/find.texi
+++ b/doc/find.texi
@@ -2826,7 +2826,7 @@ database is zero.  The second byte is immediately followed by the
 first database entry.  The first entry in the database is not preceded
 by any differential count or dummy entry.  Instead the differential
 count for the first item is assumed to be zero.
-.P
+
 Starting with the second entry (if any) in the database, data is
 interpreted as for the GNU LOCATE02 format.
 
@@ -3329,16 +3329,6 @@ that ``exports'' configurations for variable definitions and uses
 another shell script that ``sources'' the configuration file into the
 environment and then executes @code{updatedb} in the environment.
 
-@code{updatedb} creates and updates the database of file names used by
-@code{locate}.  @code{updatedb} generates a list of files similar to
-the output of @code{find} and then uses utilities for optimizing the
-database for performance.  @code{updatedb} is often run periodically
-as a @code{cron} job and configured with environment variables or
-command options.  Typically, operating systems have a shell script
-that ``exports'' configurations for variable definitions and uses
-another shell script that ``sources'' the configuration file into the
-environment and then executes @code{updatedb} in the environment.
-
 @table @code
 @item --findoptions='@var{OPTION}@dots{}'
 Global options to pass on to @code{find}.
--
1.6.4

[Peter Breitenlohner]

Hi,

the handling of slocate databases in locate.c is (1) unnecessarily
complicated, and (2) assumes that the second path is an extension of the
first one.  This may or may not be true.

Attached are two patches:

0002-slocate-*: adds a test case exposing the bug.

0003-slocate-*: simplifies the handling of slocate databases, also fixing
the bug.

Regards
Peter Breitenlohner <peb@...>
From a3a36a2dc9e04b7f7c8425c1888ebb6670944018 Mon Sep 17 00:00:00 2001
From: Peter Breitenlohner <peb@...>
Date: Mon, 3 Aug 2009 13:10:21 +0200
Subject: [PATCH 2/5] slocate: add a testcase and expose bug

Signed-off-by: Peter Breitenlohner <peb@...>
---
 locate/testsuite/Makefile.am            |    2 ++
 locate/testsuite/locate.gnu/slocate.exp |    9 +++++++++
 locate/testsuite/locate.gnu/slocate.xo  |    3 +++
 3 files changed, 14 insertions(+), 0 deletions(-)
 create mode 100644 locate/testsuite/locate.gnu/slocate.exp
 create mode 100644 locate/testsuite/locate.gnu/slocate.xo

diff --git a/locate/testsuite/Makefile.am b/locate/testsuite/Makefile.am
index 75623bc..ca07ba8 100644
--- a/locate/testsuite/Makefile.am
+++ b/locate/testsuite/Makefile.am
@@ -17,6 +17,7 @@ locate.gnu/regex1.exp \
 locate.gnu/exists1.exp \
 locate.gnu/exists2.exp \
 locate.gnu/exists3.exp \
+locate.gnu/slocate.exp \
 locate.gnu/notexists1.exp \
 locate.gnu/notexists2.exp \
 locate.gnu/notexists3.exp \
@@ -38,6 +39,7 @@ locate.gnu/ignore_case3.xo \
 locate.gnu/exists1.xo \
 locate.gnu/exists2.xo \
 locate.gnu/exists3.xo \
+locate.gnu/slocate.xo \
 locate.gnu/notexists1.xo \
 locate.gnu/notexists2.xo \
 locate.gnu/notexists3.xo \
diff --git a/locate/testsuite/locate.gnu/slocate.exp b/locate/testsuite/locate.gnu/slocate.exp
new file mode 100644
index 0000000..c36d99d
--- /dev/null
+++ b/locate/testsuite/locate.gnu/slocate.exp
@@ -0,0 +1,9 @@
+# tests an slocate database, string1 (tmp/subdia) is not prefix of string2 (tmp/subdir).
+set tmp "tmp"
+exec rm -rf $tmp
+exec mkdir $tmp
+exec touch $tmp/subdia
+exec mkdir $tmp/subdir
+exec touch $tmp/subdir/fred
+locate_start p "--changecwd=. --output=$tmp/locatedb --dbformat=slocate --localpaths=$tmp/subdi*" "--database=$tmp/locatedb -e subdi 2>/dev/null" {}
+
diff --git a/locate/testsuite/locate.gnu/slocate.xo b/locate/testsuite/locate.gnu/slocate.xo
new file mode 100644
index 0000000..ad0208c
--- /dev/null
+++ b/locate/testsuite/locate.gnu/slocate.xo
@@ -0,0 +1,3 @@
+tmp/subdia
+tmp/subdir
+tmp/subdir/fred
--
1.6.4


From c0b597a760ea6fc1ff80e9e2543645b781c6b3e2 Mon Sep 17 00:00:00 2001
From: Peter Breitenlohner <peb@...>
Date: Mon, 3 Aug 2009 13:23:42 +0200
Subject: [PATCH 3/5] slocate: simplify and bug fix


Signed-off-by: Peter Breitenlohner <peb@...>
---
 ChangeLog       |    8 ++++++++
 locate/locate.c |   50 +++++++++-----------------------------------------
 2 files changed, 17 insertions(+), 41 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index fd14c44..caffb69 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-08-03  Peter Breitenlohner  <peb@...>
 
+ * locate/locate.c: Simplify handling of slocate databases.
+ Fix the bug assuming second path extends the first one.
+ * locate/testsuite/locate.gnu/slocate.{exp,xo}: New testcase
+ for slocate databse, exposing this bug.
+ * locate/testsuite/Makefile.am: Add new test case.
+
[phone]  Peter Breitenlohner  <peb@...>
+
  * doc/find.texi: Remove duplicate paragraph and fix a typo.
 
 2009-07-18  James Youngman  <jay@...>
diff --git a/locate/locate.c b/locate/locate.c
index c01f6c2..4f2b1a7 100644
--- a/locate/locate.c
+++ b/locate/locate.c
@@ -330,7 +330,6 @@ struct regular_expression
 struct process_data
 {
   int c; /* An input byte.  */
-  char itemcount;        /* Indicates we're at the beginning of an slocate db. */
   int count; /* The length of the prefix shared with the previous database entry.  */
   int len;
   char *original_filename; /* The current input database entry. */
@@ -338,7 +337,6 @@ struct process_data
   char *munged_filename; /* path or basename(path) */
   FILE *fp; /* The pathname database.  */
   const char *dbfile; /* Its name, or "<stdin>" */
-  int  slocatedb_format; /* Allows us to cope with slocate's format variant */
   GetwordEndianState endian_state;
   /* for the old database format,
      the first and second characters of the most common bigrams.  */
@@ -565,37 +563,12 @@ visit_locate02_format(struct process_data *procdata, void *context)
   int nread;
   (void) context;
 
-  if (procdata->slocatedb_format)
-    {
-      if (procdata->itemcount == 0)
- {
-  ungetc(procdata->c, procdata->fp);
-  procdata->count = 0;
-  procdata->len = 0;
- }
-      else if (procdata->itemcount == 1)
- {
-  procdata->count = procdata->len-1;
- }
-      else
- {
-  if (procdata->c == LOCATEDB_ESCAPE)
-    procdata->count += (short)get_short (procdata->fp);
-  else if (procdata->c > 127)
-    procdata->count += procdata->c - 256;
-  else
-    procdata->count += procdata->c;
- }
-    }
-  else
-    {
       if (procdata->c == LOCATEDB_ESCAPE)
  procdata->count += (short)get_short (procdata->fp);
       else if (procdata->c > 127)
  procdata->count += procdata->c - 256;
       else
  procdata->count += procdata->c;
-    }
 
   if (procdata->count > procdata->len || procdata->count < 0)
     {
@@ -622,16 +595,6 @@ visit_locate02_format(struct process_data *procdata, void *context)
 
   procdata->munged_filename = procdata->original_filename;
 
-  if (procdata->slocatedb_format)
-    {
-      /* Don't increment indefinitely, it might overflow. */
-      if (procdata->itemcount < 6)
- {
-  ++(procdata->itemcount);
- }
-    }
-
-
   return VISIT_CONTINUE;
 }
 
@@ -1065,6 +1028,7 @@ search_one_database (int argc,
   struct process_data procdata; /* Storage for data shared with visitors. */
   int slocate_seclevel;
   int oldformat;
+  int slocatedb_format;
   struct visitor* pvis; /* temp for determining past_pat_inspector. */
   const char *format_name;
   enum ExistenceCheckType do_check_existence;
@@ -1085,8 +1049,6 @@ search_one_database (int argc,
   oldformat = 0;
   procdata.endian_state = GetwordEndianStateInitial;
   procdata.len = procdata.count = 0;
-  procdata.slocatedb_format = 0;
-  procdata.itemcount = 0;
 
   procdata.dbfile = dbfile;
   procdata.fp = fp;
@@ -1164,13 +1126,13 @@ search_one_database (int argc,
  }
       add_visitor(visit_locate02_format, NULL);
       format_name = "slocate";
-      procdata.slocatedb_format = 1;
+      slocatedb_format = 1;
     }
   else
     {
       int nread2;
 
-      procdata.slocatedb_format = 0;
+      slocatedb_format = 0;
       extend (&procdata, sizeof(LOCATEDB_MAGIC), 0u);
       nread2 = fread (procdata.original_filename+nread, 1, sizeof (LOCATEDB_MAGIC)-nread,
       procdata.fp);
@@ -1352,6 +1314,12 @@ search_one_database (int argc,
 
 
   procdata.c = getc (procdata.fp);
+  if (slocatedb_format  && (procdata.c != EOF))
+    {
+      /* Make slocate database look like GNU locate database. */
+      ungetc(procdata.c, procdata.fp);
+      procdata.c = 0;
+    }
   /* If we are searching for filename patterns, the inspector list
    * will contain an entry for each pattern for which we are searching.
    */
--
1.6.4

[Peter Breitenlohner]

Hi,

in locate.c assertions are disabled, because they would fail due to various
off-by-one bugs.

Attached are two patches addressing this:

0004-locate-*: enable assertions to expose the bugs -- many test will FAIL.

0005-locate-*: fix the bugs such that all tests PASS.

I sincerely hope this and my two previous messages (together 5 patches) will
make it into findutils-4.4.X.

Regards
Peter Breitenlohner <peb@...>
From a571287a155bfa103fe7041a644defefa52f3fd9 Mon Sep 17 00:00:00 2001
From: Peter Breitenlohner <peb@...>
Date: Mon, 3 Aug 2009 13:30:29 +0200
Subject: [PATCH 4/5] locate: enable assertions to expose bug


Signed-off-by: Peter Breitenlohner <peb@...>
---
 locate/locate.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/locate/locate.c b/locate/locate.c
index 4f2b1a7..c32db70 100644
--- a/locate/locate.c
+++ b/locate/locate.c
@@ -81,7 +81,6 @@
 
 #include <fcntl.h>
 
-#define NDEBUG
 #include <assert.h>
 #include <string.h>
 
--
1.6.4


From 33b3bb4e10ee96bfe5039adffd1cfaa6d2cc23b9 Mon Sep 17 00:00:00 2001
From: Peter Breitenlohner <peb@...>
Date: Mon, 3 Aug 2009 13:34:08 +0200
Subject: [PATCH 5/5] locate: fix off-by-one bugs and enable assertions


Signed-off-by: Peter Breitenlohner <peb@...>
---
 ChangeLog       |    5 +++++
 locate/locate.c |   17 ++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index caffb69..6e0c530 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2009-08-03  Peter Breitenlohner  <peb@...>
 
+ * locate/locate.c: Enable assertions and fix the off-by-one
+ bugs that prevented this.
+
[phone]  Peter Breitenlohner  <peb@...>
+
  * locate/locate.c: Simplify handling of slocate databases.
  Fix the bug assuming second path extends the first one.
  * locate/testsuite/locate.gnu/slocate.{exp,xo}: New testcase
diff --git a/locate/locate.c b/locate/locate.c
index c32db70..d73354c 100644
--- a/locate/locate.c
+++ b/locate/locate.c
@@ -293,7 +293,7 @@ locate_read_str(char **buf, size_t *siz, FILE *fp, int delimiter, int offs)
       *buf = pnew;
     }
  }
-      memcpy((*buf)+offs, p, nread);
+      memcpy((*buf)+offs, p, nread + 1);
       free(p);
     }
   return nread;
@@ -583,10 +583,21 @@ visit_locate02_format(struct process_data *procdata, void *context)
   nread = locate_read_str (&procdata->original_filename,
    &procdata->pathsize,
    procdata->fp, 0, procdata->count);
-  if (nread < 0)
+  if (nread < 1)
     return VISIT_ABORT;
   procdata->c = getc (procdata->fp);
-  procdata->len = procdata->count + nread;
+  procdata->len = procdata->count + nread - 1; /* Number of chars in path. */
+
+  if (procdata->len < 1)
+    {
+      /* This should not happen generally, but since we're
+       * reading in data which is outside our control, we
+       * cannot prevent it.
+       */
+      error(1, 0, _("locate database %s is corrupt or invalid"),
+    quotearg_n_style(0, locale_quoting_style, procdata->dbfile));
+    }
+
   s = procdata->original_filename + procdata->len - 1; /* Move to the last char in path.  */
   assert (s[0] != '\0');
   assert (s[1] == '\0'); /* Our terminator.  */
--
1.6.4

[Peter Breitenlohner]

Hi,

one test -- test-dup2 -- recently added to the findutils test suite is
inherently broken and may fail.

The test (from gnulib) assumes that after (test-dup2.c line 68)
  int fd = open (file, O_CREAT | O_RDWR, 0600);
fd is an open file (correct), and that fd+1, fd+2 are not (wrong, that may
or may not be true).

E.g., when running the testsuite under Midnight Commander (mc), originally the open
files are 0, 1, 2, 4, 6, 8, and 255. Thus fd=3, and fd+1 is open.

Unless the test programs first closes some files, there is no guarantee that
the test does the right thing.

PLEASE: remove that test and report the problem to the gnulib maintainers.

Regards
Peter Breitenlohner <peb@...>

[Eric Blake-2]

> one test -- test-dup2 -- recently added to the findutils test suite
> is
> inherently broken and may fail.

Thanks for the report.

>
> The test (from gnulib) assumes that after (test-dup2.c line 68)
>   int fd = open (file, O_CREAT | O_RDWR, 0600);
> fd is an open file (correct), and that fd+1, fd+2 are not (wrong, that
> may
> or may not be true).
>
> E.g., when running the testsuite under Midnight Commander (mc),
> originally the open
> files are 0, 1, 2, 4, 6, 8, and 255. Thus fd=3, and fd+1 is open.

Good catch.

> PLEASE: remove that test and report the problem to the gnulib
> maintainers.

Rather than removing the test, let's fix it by closing the next
two fd's.  It should be an easy enough fix, but I don't have
commit access right now or I would do it.  And you can report
this directly to the gnulib maintainers yourself (as I just did
by adding a cc).  The ASSERT on line 72 was designed to catch
testsuite portability problems as much as platform bugs.

Meanwhile, consider writing a bug report against mc for letting
so many fd's leak into child processes, whether or not those fd's
have any bearing on the children.  In some regards, leaking fd's
can form security weaknesses.

--
Eric Blake

[Peter Breitenlohner]

On Mon, 3 Aug 2009, Peter Breitenlohner wrote:

> the handling of slocate databases in locate.c is (1) unnecessarily
> complicated, and (2) assumes that the second path is an extension of the
> first one.  This may or may not be true.
>
> Attached are two patches:

Hi,

sorry, this should have been three patches.

> 0002-slocate-*: adds a test case exposing the bug.
>
> 0003-slocate-*: simplifies the handling of slocate databases, also fixing
> the bug.

Here the missing patch:

0006-slocate-*: reindent and fix a typo.

Regards
Peter Breitenlohner <peb@...>
From ea5cd4527b8417cbf1eaf67092e9b853b7802830 Mon Sep 17 00:00:00 2001
From: Peter Breitenlohner <peb@...>
Date: Tue, 4 Aug 2009 10:42:18 +0200
Subject: [PATCH 6/6] slocate: reindent and fix typo


Signed-off-by: Peter Breitenlohner <peb@...>
---
 locate/locate.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/locate/locate.c b/locate/locate.c
index d73354c..804086b 100644
--- a/locate/locate.c
+++ b/locate/locate.c
@@ -562,16 +562,16 @@ visit_locate02_format(struct process_data *procdata, void *context)
   int nread;
   (void) context;
 
-      if (procdata->c == LOCATEDB_ESCAPE)
- procdata->count += (short)get_short (procdata->fp);
-      else if (procdata->c > 127)
- procdata->count += procdata->c - 256;
-      else
- procdata->count += procdata->c;
+  if (procdata->c == LOCATEDB_ESCAPE)
+    procdata->count += (short)get_short (procdata->fp);
+  else if (procdata->c > 127)
+    procdata->count += procdata->c - 256;
+  else
+    procdata->count += procdata->c;
 
   if (procdata->count > procdata->len || procdata->count < 0)
     {
-      /* This should not happen generally , but since we're
+      /* This should not happen generally, but since we're
        * reading in data which is outside our control, we
        * cannot prevent it.
        */
--
1.6.4