fts_open() (was: Re: Patch to restore WARNS feature)
Should I commit the attached patch then?
On Tue, Jun 12, 2001 at 02:35:39PM +1000, Bruce Evans wrote:
For those interested, here's the missing patch.
Index: lib/libc/gen/fts.c
===
RCS file: /home/ncvs/src/lib/libc/gen/fts.c,v
retrieving revision 1.18
diff -u -p -r1.18 fts.c
--- lib/libc/gen/fts.c2001/06/01 21:53:50 1.18
+++ lib/libc/gen/fts.c2001/06/11 18:20:17
@@ -936,7 +936,8 @@ fts_sort(sp, head, nitems)
}
for (ap = sp-fts_array, p = head; p; p = p-fts_link)
*ap++ = p;
- qsort((void *)sp-fts_array, nitems, sizeof(FTSENT *), sp-fts_compar);
+ qsort((void *)sp-fts_array, nitems, sizeof(FTSENT *),
+ (int (*) __P((const void *, const void *)))sp-fts_compar);
for (head = *(ap = sp-fts_array); --nitems; ++ap)
ap[0]-fts_link = ap[1];
ap[0]-fts_link = NULL;
This just hides the bug that fts's comparison function is not suitable
for use by qsort().
Bruce
On Tue, Jun 12, 2001 at 02:38:13PM -0400, Garrett Wollman wrote:
On Tue, 12 Jun 2001 15:53:18 +0300, Ruslan Ermilov [EMAIL PROTECTED] said:
+ qsort((void *)sp-fts_array, nitems, sizeof(FTSENT *),
+ (int (*) __P((const void *, const void *)))sp-fts_compar);
This is wrong. The declaration of the comparison function should be
fixed, rather than papering over the mistake here.
(This is arguably a deficiency in the C standard. qsort() should take
an additional state parameter for the comparison function, but
doesn't.)
--
Ruslan Ermilov Oracle Developer/DBA,
[EMAIL PROTECTED] Sunbay Software AG,
[EMAIL PROTECTED] FreeBSD committer,
+380.652.512.251Simferopol, Ukraine
http://www.FreeBSD.org The Power To Serve
http://www.oracle.com Enabling The Information Age
Index: include/fts.h
===
RCS file: /home/ncvs/src/include/fts.h,v
retrieving revision 1.3
diff -u -p -r1.3 fts.h
--- include/fts.h 1997/05/07 19:59:58 1.3
+++ include/fts.h 2001/06/13 12:17:31
@@ -45,7 +45,8 @@ typedef struct {
int fts_rfd;/* fd for root */
int fts_pathlen;/* sizeof(path) */
int fts_nitems; /* elements in the sort array */
- int (*fts_compar)();/* compare function */
+ int (*fts_compar) /* compare function */
+ __P((const void *, const void *));
#defineFTS_COMFOLLOW 0x001 /* follow command line symlinks */
#defineFTS_LOGICAL 0x002 /* logical walk */
@@ -120,7 +121,7 @@ __BEGIN_DECLS
FTSENT *fts_children __P((FTS *, int));
int fts_close __P((FTS *));
FTS*fts_open __P((char * const *, int,
- int (*)(const FTSENT **, const FTSENT **)));
+ int (*)(const void *, const void *)));
FTSENT *fts_read __P((FTS *));
int fts_set __P((FTS *, FTSENT *, int));
__END_DECLS
Index: lib/libc/gen/fts.3
===
RCS file: /home/ncvs/src/lib/libc/gen/fts.3,v
retrieving revision 1.12
diff -u -p -r1.12 fts.3
--- lib/libc/gen/fts.3 2001/02/01 16:29:34 1.12
+++ lib/libc/gen/fts.3 2001/06/13 12:17:32
@@ -45,7 +45,7 @@
.Fd #include sys/stat.h
.Fd #include fts.h
.Ft FTS *
-.Fn fts_open char * const *path_argv int options int (*compar)(const FTSENT **,
const FTSENT **)
+.Fn fts_open char * const *path_argv int options int (*compar)(const void *,
+const void *)
.Ft FTSENT *
.Fn fts_read FTS *ftsp
.Ft FTSENT *
@@ -462,9 +462,9 @@ The argument
specifies a user-defined function which may be used to order the traversal
of the hierarchy.
It
-takes two pointers to pointers to
-.Fa FTSENT
-structures as arguments and
+takes two pointers as arguments (which need to be cast to
+.Vt FTSENT **
+inside the function's body) and
should return a negative value, zero, or a positive value to indicate
if the file referenced by its first argument comes before, in any order
with respect to, or after, the file referenced by its second argument.
Index: lib/libc/gen/fts.c
===
RCS file: /home/ncvs/src/lib/libc/gen/fts.c,v
retrieving revision 1.18
diff -u -p -r1.18 fts.c
--- lib/libc/gen/fts.c 2001/06/01 21:53:50 1.18
+++ lib/libc/gen/fts.c 2001/06/13 12:17:35
@@ -85,7 +85,7 @@ FTS *
fts_open(argv, options, compar)
char * const *argv;
register int options;
- int (*compar) __P((const FTSENT **, const FTSENT **));
+ int (*compar) __P((const void *, const void *));
{
register FTS *sp;
register FTSENT *p, *root;
Index: bin/cp/cp.c
===
RCS file: /home/ncvs/src/bin/cp/cp.c,v
retrieving revision 1.26
diff -u -p -r1.26 cp.c
--- bin/cp/cp.c 2001/06/11 13:57:54 1.26
+++
Re: fts_open() (was: Re: Patch to restore WARNS feature)
On Wed, 13 Jun 2001, Ruslan Ermilov wrote: Should I commit the attached patch then? I don't like changing the documented interface of fts_open() but all the alternatives that I can think of aren't appealing: 1. Provide a glue function that converts what qsort expects to the documented interface. This seems to require saving sp-fts_compare in a global variable so that it can be accessed in the glue function. 2. Provide an alternative to qsort() that takes an fts-compatible comparison function. 3. Provide an alternative to qsort() that takes an comparison function that takes an additional function pointer arg (use this arg to avoid the global in (1)). Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: fts_open() (was: Re: Patch to restore WARNS feature)
On Wed, 13 Jun 2001 23:28:29 +1000 (EST), Bruce Evans [EMAIL PROTECTED] said: 3. Provide an alternative to qsort() that takes an comparison function that takes an additional function pointer arg (use this arg to avoid the global in (1)). Actually, doing this would solve a number of similar problems, and it doesn't look to be too difficult to do. I would make the additional argument a `void *'; fts's comparison trampoline function could use this as a `FTS *' to look up the comparison function. (I think this is more general than passing any kind of function pointer.) -GAWollman To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: fts_open() (was: Re: Patch to restore WARNS feature)
On Wed, Jun 13, 2001 at 11:16:57AM -0400, Garrett Wollman wrote: On Wed, 13 Jun 2001 23:28:29 +1000 (EST), Bruce Evans [EMAIL PROTECTED] said: 3. Provide an alternative to qsort() that takes an comparison function that takes an additional function pointer arg (use this arg to avoid the global in (1)). Actually, doing this would solve a number of similar problems, and it doesn't look to be too difficult to do. I would make the additional argument a `void *'; fts's comparison trampoline function could use this as a `FTS *' to look up the comparison function. (I think this is more general than passing any kind of function pointer.) How should we call this function? (I'll implement this tomorrow.) -- Ruslan Ermilov Oracle Developer/DBA, [EMAIL PROTECTED] Sunbay Software AG, [EMAIL PROTECTED] FreeBSD committer, +380.652.512.251Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: fts_open() (was: Re: Patch to restore WARNS feature)
On Wed, 13 Jun 2001 11:55:30 -0400, Alfred Perlstein [EMAIL PROTECTED] said: Why not do something like the rpc code does? Check if threaded, if so cons up a thread specific key otherwise use a global. The Standard does not appear to say whether qsort() is reentrant, but I believe that it ought to be. fts() was clearly intended to be reentrant. (I don't know what good it would do, but let's not make things worse than they already are, hey?) -GAWollman To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: fts_open() (was: Re: Patch to restore WARNS feature)
On Wed, 13 Jun 2001, Garrett Wollman wrote: On Wed, 13 Jun 2001 23:28:29 +1000 (EST), Bruce Evans [EMAIL PROTECTED] said: 3. Provide an alternative to qsort() that takes an comparison function that takes an additional function pointer arg (use this arg to avoid the global in (1)). Actually, doing this would solve a number of similar problems, and it doesn't look to be too difficult to do. I would make the additional argument a `void *'; fts's comparison trampoline function could use this as a `FTS *' to look up the comparison function. (I think this is more general than passing any kind of function pointer.) Good idea. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
