[PATCH 1/2] fts: do not use the getcwdat module

[Kamil Dudka]

... because there is no such module in gnulib
---
lib/fts.c | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/lib/fts.c b/lib/fts.c
index bfa73e31e..4195f6170 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -253,7 +253,6 @@ static int fts_safe_changedir (FTS *, FTSENT *, int, const char *)
# include <inttypes.h>
# include <stdint.h>
# include <stdio.h>
-# include "getcwdat.h"
bool fts_debug = false;
# define Dprintf(x) do { if (fts_debug) printf x; } while (false)
#else
@@ -1734,23 +1733,14 @@ fd_ring_print (FTS const *sp, FILE *stream, char const *msg)
{
I_ring const *fd_ring = &sp->fts_fd_ring;
unsigned int i = fd_ring->fts_front;
- char *cwd = getcwdat (sp->fts_cwd_fd, NULL, 0);
- fprintf (stream, "=== %s ========== %s\n", msg, cwd);
- free (cwd);
+ fprintf (stream, "=== %s ========== %d\n", msg, sp->fts_cwd_fd);
if (i_ring_empty (fd_ring))
return;

while (true)
{
int fd = fd_ring->fts_fd_ring[i];
- if (fd < 0)
- fprintf (stream, "%d: %d:\n", i, fd);
- else
- {
- char *wd = getcwdat (fd, NULL, 0);
- fprintf (stream, "%d: %d: %s\n", i, fd, wd);
- free (wd);
- }
+ fprintf (stream, "%d: %d:\n", i, fd);
if (i == fd_ring->fts_back)
break;
i = (i + I_RING_SIZE - 1) % I_RING_SIZE;
@@ -1770,9 +1760,7 @@ fd_ring_check (FTS const *sp)

int cwd_fd = sp->fts_cwd_fd;
cwd_fd = fcntl (cwd_fd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1);
- char *dot = getcwdat (cwd_fd, NULL, 0);
- error (0, 0, "===== check ===== cwd: %s", dot);
- free (dot);
+ error (0, 0, "===== check ===== cwd: %d", cwd_fd);
while ( ! i_ring_empty (&fd_w))
{
int fd = i_ring_pop (&fd_w);
@@ -1787,12 +1775,8 @@ fd_ring_check (FTS const *sp)
}
if (!same_fd (fd, parent_fd))
{
- char *cwd = getcwdat (fd, NULL, 0);
- error (0, errno, "ring : %s", cwd);
- char *c2 = getcwdat (parent_fd, NULL, 0);
- error (0, errno, "parent: %s", c2);
- free (cwd);
- free (c2);
+ error (0, errno, "ring : %d", fd);
+ error (0, errno, "parent: %d", parent_fd);
fts_assert (0);
}
close (cwd_fd);

[Kamil Dudka]

---
lib/fts.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/fts.c b/lib/fts.c
index 4195f6170..7983320b7 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -253,8 +253,11 @@ static int fts_safe_changedir (FTS *, FTSENT *, int, const char *)
# include <inttypes.h>
# include <stdint.h>
# include <stdio.h>
+# include <error.h>
bool fts_debug = false;
# define Dprintf(x) do { if (fts_debug) printf x; } while (false)
+static void fd_ring_print (FTS const *sp, FILE *stream, char const *msg);
+static void fd_ring_check (FTS const *sp);
#else
# define Dprintf(x)
# define fd_ring_check(x)
@@ -1732,16 +1735,18 @@ static void
fd_ring_print (FTS const *sp, FILE *stream, char const *msg)
{
I_ring const *fd_ring = &sp->fts_fd_ring;
- unsigned int i = fd_ring->fts_front;
+ unsigned int i = fd_ring->ir_front;
+ if (!fts_debug)
+ return;
fprintf (stream, "=== %s ========== %d\n", msg, sp->fts_cwd_fd);
if (i_ring_empty (fd_ring))
return;

while (true)
{
- int fd = fd_ring->fts_fd_ring[i];
+ int fd = fd_ring->ir_data[i];
fprintf (stream, "%d: %d:\n", i, fd);
- if (i == fd_ring->fts_back)
+ if (i == fd_ring->ir_back)
break;
i = (i + I_RING_SIZE - 1) % I_RING_SIZE;
}

[Paul Eggert]

Thanks, these two patches look like a good idea to me. Please give Jim a
couple of days to respond before applying, though. (Are you a Gnulib
member on savannah? If not, please apply to become one.) Also, please
read this:

https://lists.gnu.org/r/bug-gnulib/2011-07/msg00355.html

and mention and cite it in the ChangeLog entry for the relevant patch.

[Jim Meyering]

Post by Kamil Dudka
... because there is no such module in gnulib
---
lib/fts.c | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/lib/fts.c b/lib/fts.c
index bfa73e31e..4195f6170 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -253,7 +253,6 @@ static int fts_safe_changedir (FTS *, FTSENT *, int, const char *)
# include <inttypes.h>
# include <stdint.h>
# include <stdio.h>
-# include "getcwdat.h"

Yikes!
Hi Kamil,
Thanks for noticing and fixing that.

It took some digging, but I found the code that I once used (back in
2006!) when I was building/testing that with -DFTS_DEBUG.
It was in a CVS-checked-out directory I had renamed to gnulib-corrupt.

I'll include those files here, for reference, in case someone wants to
modernize them and to make a module out of them -- they probably
predate gnulib's modules directory. Given that you're the first to
mention this in nearly 12 years, I suspect it's better just to forget
about these two files and use your patch, though I do recall
preferring to see actual directory names (rather than nearly useless
FDs) in debugging output. Another problem: getcwdat.c was essentially
copied from some old getcwd.c-related file; if we were to use this new
module, we'd probably require that the .c file be mechanically
derived, to avoid the duplication.

[Kamil Dudka]

Post by Jim Meyering

Post by Kamil Dudka
... because there is no such module in gnulib
---
lib/fts.c | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/lib/fts.c b/lib/fts.c
index bfa73e31e..4195f6170 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -253,7 +253,6 @@ static int fts_safe_changedir (FTS *, FTSENT *,
int, const char *)>
# include <inttypes.h>
# include <stdint.h>
# include <stdio.h>
-# include "getcwdat.h"

Yikes!
Hi Kamil,
Thanks for noticing and fixing that.
It took some digging, but I found the code that I once used (back in
2006!) when I was building/testing that with -DFTS_DEBUG.
It was in a CVS-checked-out directory I had renamed to gnulib-corrupt.
I'll include those files here, for reference, in case someone wants to
modernize them and to make a module out of them -- they probably
predate gnulib's modules directory. Given that you're the first to
mention this in nearly 12 years, I suspect it's better just to forget
about these two files and use your patch, though I do recall
preferring to see actual directory names (rather than nearly useless
FDs) in debugging output. Another problem: getcwdat.c was essentially
copied from some old getcwd.c-related file; if we were to use this new
module, we'd probably require that the .c file be mechanically
derived, to avoid the duplication.

Hi Jim,

thank you for digging it up! As you suggest, it is really beyond my time
budget for this to make a proper gnulib module out of the original code.

My intention was to just get the current code of FTS_DEBUG compile so that
we have something to start with. I will reference your post in the commit
to let other developers make the debugging output more user-friendly again
if they have enough time and skills for that.

The reason why I decided to use FTS_DEBUG now are the following issues
reported to Red Hat Bugzilla recently:

https://bugzilla.redhat.com/1544392
https://bugzilla.redhat.com/1544429
https://bugzilla.redhat.com/1558249

Kamil