Re: Feature Branch Solaris Build Broken - lib/glob.c errors

[Derek Price]

Conrad T. Pino wrote:

From: Derek Price [mailto:[EMAIL PROTECTED]

I think I just checked in fixes for both Solaris compile problems, this
one and one with getpwnam_r.  Could you let me know if they work?  I
have no way to test here.



The compile completes but we have warnings (see below).  Problem may
be struct stat definition and I've include this platform's definition.
  


This should be irrelevant, since all reference to struct stat should
be made using the Solaris definition.  Our code never attempts to define
it and I am fairly certain that the struct stat; line in glob_.h only
declares the existance of said structure.

 719: ? ((*pglob-gl_stat) (dirname, st) == 0
1066:? (*pglob-gl_stat) (fullname, st) == 0  S_ISDIR (st.st_mode)
1116: ? (*pglob-gl_stat) (fullname, st)

gcc -DHAVE_CONFIG_H -I. -I. -I..-Ino/include  -g -O2 -c glob.c
glob.c: In function `rpl_glob':
glob.c:719: warning: passing arg 2 of pointer to function from incompatible 
pointer type
glob.c: In function `is_dir_p':
glob.c:1066: warning: passing arg 2 of pointer to function from incompatible 
pointer type
glob.c: In function `glob_in_dir':
glob.c:1116: warning: passing arg 2 of pointer to function from incompatible 
pointer type
  


I am more suspicious of the __restrict keyword.  restrict should not
cause this sort of warning by my understanding of the C99 spec, but is
it possible that this is doing something we don't understand on
Solaris?  Here's the prototype of those function calls:

#ifdef __USE_GNU
int (*gl_lstat) (__const char *__restrict, struct stat *__restrict);
int (*gl_stat) (__const char *__restrict, struct stat *__restrict);
#else
int (*gl_lstat) (__const char *__restrict, void *__restrict);
int (*gl_stat) (__const char *__restrict, void *__restrict);
#endif

So, I am guessing that the problem is here.  ST is definitately a
struct stat in all three of the calls your compiler is warning about. 
We should also be able to assume __USE_GNU is set by the line at the
beginning of glob_.h.  Even if this wasn't so, by C89 any pointer type
should be castable to void *, so the problem should still be with the
restrict keyword.

Regards,

Derek



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

GZip level restriction

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hey all,

I discussed adding a compression level restriction on the server as a
work-around for another bug that has since been fixed a few months back.

I can't find the email, but I believe Mark Baushke suggested a
restriction format like:

AllowedGZipLevels=0123456789

where 0 meant allowing no compression, and failing when the level
requested by the client was not in the list.

I think I actually prefer something more like:

DefaultGZipLevel=6
MinGZipLevel=3
MaxGZipLevel=6

and code that says:

if (level == 0) level = DefaultGZipLevel;
if (level  MinGZipLevel) level = MinGZipLevel;
if (level  MaxGZipLevel) level = MaxGZipLevel;

In preference to actually issuing an error, though perhaps a warning
would still be appropriate.  This way, users that put cvs -z9 in their
.cvsrc will always get the maximum gzip level allowed by any server and
still be able to use their .cvsrc with all the servers they access. 
Also, decompression is much less CPU intensive than compression, so it
shouldn't hurt to let the client compress at whatever level it likes and
clients should still use level 9 for compression in this scenario, and
the server would use whatever was declared its max level.

Just wanted to run this by the list before I begin coding.  I suppose
that all four config keywords could be added.  Then, default, min, and
max could be processed as I am suggesting and AllowedGZipLevels could be
used if an admin wanted their server to emit real errors for some
reason.  I'm not sure I see the utility here, though, so I'd just as
soon leave it out unless someone can come up with a believable scenario
which demonstrates its utility.

Cheers,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCjiRpLD1OTBfyMaQRApggAKDewo8mJtB5OEiYRn3Dtt1o9Ljm0QCeIZEY
SC55YocLuAh4P8bMPQN6KaA=
=y45z
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: GZip level restriction

[Larry Jones]

Derek Price writes:
 
 I discussed adding a compression level restriction on the server as a
 work-around for another bug that has since been fixed a few months back.

I don't think hard restrictions are a good idea.  The best compression
level depends on the total system: the server machine, the client
machine, and the communication path between them.  The server doesn't
know enough about the whole system to impose absolute restrictions.  If
I'm at the other end of a 300bps phone line, I want -z9 no matter how
much CPU time it takes.  If I'm trying to debug the connection, I want
-z0 even if the server is at the other end of a 300bps phone line. 
Since the client is most likely to have an intelligent operator behind
it who *can* determine the properties of the entire system, I think the
server should always try to provide what the client asks for.

On the other hand, it would be nice if there were a way to specify a
default compression level or range of levels (both for the server and
the client) that could be used in the absence of any explicit
specification by the user.  Of course, reconciling conflicting defaults
could be an interesting challenge.

-Larry Jones

Philistines. -- Calvin


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: GZip level restriction

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Larry Jones wrote:

Derek Price writes:

I discussed adding a compression level restriction on the server as a
work-around for another bug that has since been fixed a few months back.


I don't think hard restrictions are a good idea. The best compression
level depends on the total system: the server machine, the client
machine, and the communication path between them. The server doesn't
know enough about the whole system to impose absolute restrictions. If
I'm at the other end of a 300bps phone line, I want -z9 no matter how
much CPU time it takes. If I'm trying to debug the connection, I want
-z0 even if the server is at the other end of a 300bps phone line.
Since the client is most likely to have an intelligent operator behind
it who *can* determine the properties of the entire system, I think the
server should always try to provide what the client asks for.


I don't agree.  First off, once Min and Max configs are implemented, an
adminstrator who agrees with you can simply not touch their config file
or explicitly set MinCompressionLevel=0 and MaxCompressionLevel=9 and
allow users to make their own decision about what level is best.

Secondly, if I am an administrator running a 200Mhz server behind a
high-speed DSL, I might want to set MaxCompressionLevel=0 or 1 depending
on how I felt about hard errors and avoid trusting any tom, dick or
harry to get it right and not bring my server to a crawl.  This actually
applies to an admin running a 3GHz server behind a T1 with enough users
that -z9 can still bog down the system when 20 of them hit the server at
once.  A max compression level of, say, 3, might double the number of
clients that can hit the server before it reaches maximum load.

This is especially relevant for admins dealing with lazy users out there
like me who run on fast client machines behind a medium-speed DSL and
lazily specify -z9 in their .cvsrc for all servers.

Similarly, if my server is up on a fast machine behind a slow modem,
perhaps I could confidently set my MinCompressionLevel=9.

In short, there are scenarios where it makes sense for the server admin
to want to restrict the compresssion level, this feature wouldn't be
disabling any old functionality, and I am offering to do the work. 
Where is the harm?

On the other hand, it would be nice if there were a way to specify a
default compression level or range of levels (both for the server and
the client) that could be used in the absence of any explicit
specification by the user. Of course, reconciling conflicting defaults
could be an interesting challenge.


To some extent this would be happening with my proposed change.  A
client that had requested -z9 on a fast machine would always compress
their data at level 9, but the server would only compress its data at
whatever maximum level had been set, limiting the load on its CPU.

Similarly for a client -z3 and a server with a min level of 9.  The
client would compress at 3 and the server at 9, saving most the
bandwidth and not bombing the client CPU.

Regards,

Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCjlJALD1OTBfyMaQRAqeeAKDlAgwm32k1C5WoXaPjPZuv6vGXBQCfZv6+
s8/5+7gP1IHGn/ZDh7/m998=
=5OIj
-END PGP SIGNATURE-




___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Paul Eggert wrote:

There is one change I know I made that might have an effect.  I added
the !defined _LIBC to the following block because I was unsure what
__REDIRECT_NTH was supposed to be doing



I think that one's OK.
  


Larry Jones on the CVS team just made a comment that makes me wonder if
it is, at least if we still plan on sharing this header with glibc... 
isn't the conversion to 64 bit file sizes and offsets supposed to be
transparent and handled by compile-time magic?

So, all the HAVE_.*64 stuff in glob.c shouldn't be necessary when !_LIBC.

Also, this particular bit in glob_.h would need to be used when !_LIBC
as well, since user-space programs compiling against the glob.h system
header from glibc would want glob64 used as compile-time magic, correct?

Please excuse me if this is way off, I don't have much experience at the
64-bit file offset stuff, myself.  One thing that leads me to believe I
may be wrong is that it looks like the glob64 from glibc 2.3.5 is only a
stub function.

Regards,

Derek



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

RE: Feature Branch Solaris Build Broken - lib/glob.c errors

[Conrad T. Pino]

Hi Derek,

 From: Derek Price
 
 This should be irrelevant, since all reference to struct stat should
 be made using the Solaris definition.  Our code never attempts to define
 it and I am fairly certain that the struct stat; line in glob_.h only
 declares the existance of said structure.

[snip]  See below.

 I am more suspicious of the __restrict keyword.  restrict should not
 cause this sort of warning by my understanding of the C99 spec, but is
 it possible that this is doing something we don't understand on
 Solaris?  Here's the prototype of those function calls:
 
 #ifdef __USE_GNU
 int (*gl_lstat) (__const char *__restrict, struct stat *__restrict);
 int (*gl_stat) (__const char *__restrict, struct stat *__restrict);
 #else
 int (*gl_lstat) (__const char *__restrict, void *__restrict);
 int (*gl_stat) (__const char *__restrict, void *__restrict);
 #endif
 
 So, I am guessing that the problem is here.  ST is definitately a
 struct stat in all three of the calls your compiler is warning about. 
 We should also be able to assume __USE_GNU is set by the line at the
 beginning of glob_.h.  Even if this wasn't so, by C89 any pointer type
 should be castable to void *, so the problem should still be with the
 restrict keyword.

No amount of playing with __restrict made any difference.  I removed it
completely and the warning was identical.

Either this platform's struct stat implementation or the compiler's
interpretation of same are incompatible with a struct stat; forward
declaration.

The only way I could eliminate the warnings was to either:

1. #include sys/stat.h within glob_.h file.

2. Move #include glob.h below #include sys/stat.h in glob.c file.

The patch below implements the latter.  I'll commit it if you approve.

Ditto,

 Regards,

Ditto,

 Derek

Conrad

/export/home/cvsusr/ccvs/cvs-1.12:$ cvs diff -Nup lib/glob.c
Index: lib/glob.c
===
RCS file: /cvs/ccvs/lib/glob.c,v
retrieving revision 1.11
diff -u -p -r1.11 glob.c
--- lib/glob.c  20 May 2005 18:39:47 -  1.11
+++ lib/glob.c  21 May 2005 00:48:05 -
@@ -20,13 +20,13 @@
 # include config.h
 #endif
 
-#include glob.h
-
 #include errno.h
 #include sys/types.h
 #include sys/stat.h
 #include stddef.h
 
+#include glob.h
+
 /* Outcomment the following line for production quality code.  */
 /* #define NDEBUG 1 */
 #include assert.h



___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

RE: Feature Branch Solaris Build Broken - lib/glob.c errors

[Conrad T. Pino]

Excuse me for perhaps being slow but why is configure choosing to
use lib/glob.h when the platform has /usr/include/glob.h whose
contents follow?

/usr/include:$ cat glob.h
/*
 * Copyright (c) 1994 by Sun Microsystems, Inc.
 * Copyright 1985, 1992 by Mortice Kern Systems Inc.  All rights reserved.
 */

#ifndef _GLOB_H
#define _GLOB_H

#pragma ident   @(#)glob.h 1.4 95/03/08 SMI

#include sys/types.h

#ifdef  __cplusplus
extern C {
#endif

typedef struct  glob_t  {
size_t  gl_pathc;   /* Count of paths matched by pattern */
char**gl_pathv; /* List of matched pathnames */
size_t  gl_offs;/* # of slots reserved in gl_pathv */
/* following are internal to the implementation */
char**gl_pathp; /* gl_pathv + gl_offs */
int gl_pathn;   /* # of elements allocated */
}   glob_t;

/*
 * flags argument to glob function.
 */
#define GLOB_ERR0x0001  /* Don't continue on directory error */
#define GLOB_MARK   0x0002  /* Mark directories with trailing / */
#define GLOB_NOSORT 0x0004  /* Don't sort pathnames */
#define GLOB_NOCHECK0x0008  /* Return unquoted arg if no match */
#define GLOB_DOOFFS 0x0010  /* Ignore gl_offs unless set */
#define GLOB_APPEND 0x0020  /* Append to previous glob_t */
#define GLOB_NOESCAPE   0x0040  /* Backslashes do not quote M-chars */

/*
 * Error returns from glob
 */
#define GLOB_NOSYS  (-4)/* function not supported (XPG4) */
#define GLOB_NOMATCH(-3)/* Pattern does not match */
#define GLOB_NOSPACE(-2)/* Not enough memory */
#define GLOB_ABORTED(-1)/* GLOB_ERR set or errfunc return!=0 */

#if defined(__STDC__)
extern int glob(const char *, int, int(*)(const char *, int), glob_t *);
extern void globfree(glob_t *);
#else
extern int glob();
extern void globfree();
#endif

#ifdef  __cplusplus
}
#endif

#endif  /* _GLOB_H */
/usr/include:$ 


___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs