Re: CVS problem with ssh

[Derek Price]

Richard M. Stallman wrote:

>If the problem occurs with fflush, most likely it will also occur
>with fwrite, fclose, etc.  fflush is merely the guinea pig here.
>
>We may have to send bug reports for the systems where it does not always work.
>  
>

Agreed.  A failed retry in this case is no worse than before, when the
initial failure was not detected and data was lost.

Regards,

Derek



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

Re: CVS problem with ssh

[Derek Price]

Hi bug-gnulib,

I'd like to reopen a discussion on this list from last September: 
.

At the end of that discussion I didn't create the blocking-io module due
mostly to Paul Eggert's objections (and a lack of time on my part,
especially for a solution which sounded like it would be rejected). 
Anyhow, RMS wanted the problem this fixes in CVS fixed badly enough that
he wrote most of the stdio wrappers we discussed himself.  The public
portion of that new discussion is archived here:
.

In any case, after a few iterations and testing in CVS on GNU/Linux, RMS
& I came up with the two files I've attached to this email.  I believe
they answer all of the objections raised in the first thread linked
above except Bruno's comment on porting to WOE32 and the following
comments, which I will attempt to address here.  The WOE32 issue and
conversion to a proper GNULIB module can wait until the disposition of
this code is settled.

Paul Eggert wrote:

>You have to include stdio.h first, then #undef fwrite, then
>#define fwrite.  It can be done (Emacs did it, after all), but I don't
>recommend it.
>

Isn't this the standard behavior for a GNULIB module's header file? 
This is almost exactly what unlocked-io does, after all.  In any case, I
think this is now addressed by the functionality in blocking-io.h.

>/ POSIX says fflush sets errno.  I'm worried about implementations that/
>/ don't conform (or perhaps conform to an older standard?)./


In the cases where fflush does not set errno, we are no worse off than
we were before.  The way the new wrappers are written, the function
behaviors should remain unchanged if errno != EAGAIN (or EWOULDBLOCK on
BSD) and when when they do not return short writes.

>As far as I can tell POSIX doesn't guarantee that repeated fflushes
>will eventually write all the data, with no duplicates, in this
>situation.  Here's the spec:
>http://www.opengroup.org/onlinepubs/009695399/functions/fflush.html
>

Maybe not, but isn't this by far the implication of EAGAIN in most other
contexts?  Couldn't do what you asked this time, try *AGAIN*?  I can
understand the problem with fwrite and an object size > 1 since there is
no way for the function to return a partial item count, but once stdio
has the data, why should fflush not track what was written properly? 
Aside from bugs, this is what I would expect from most fflush
implementations.  How about we install the module and see what sort of
problem reports we get?

Bruno Habile wrote:

>Larry Jones wrote:
>>/ they should be testing for writability with select before trying to/
>>/ write to stderr rather than just blindly putting stderr into nonblocking/
>>/ mode/
>
>Agreed: On platforms where select() or poll() is available, there is no
>reason to use the old non-blocking I/O facility.
>

With this fix, CVS and potentially other tools will work with the broken
SSH implementations and other, hypothetical, future tools, which are as
rude in their treatment of shared file descriptors, regardless of
whether the OpenSSH folks fix this issue.


I would like to see this installed in several locations for the
following reasons, all basically various aspects of the advantages of
code-sharing:

GLIBC:

   1. This module provides useful functionality often expected by
  programs: guaranteed blocking behavior from stdio.
   2. Would promote use in other tools, avoiding similar problems in the
  future.
   3. Would be ported to more platforms more quickly, decreasing
  liklihood of broken CVS releases.
   4. More eyes on this code would mean fewer bugs.


GNULIB:

   1. All the reasons I want this in GLIBC.
   2. Ease of importing GLIBC updates to this code into CVS.


CVS:

   1. This fix certainly shouldn't hurt anything, once the portability
  issues are dealt with.
   2. Will hopefully fix interaction with OpenSSH, and potentially other
  tools, on most platforms.


I might get away with installing this just in CVS, but I am afraid that
CVS doesn't have the manpower to deal with the potential porting issues
raised by various parties in the earlier discussions without the GLIBC
and GNULIB support, and RMS said he would back getting this into GLIBC &
GNULIB to get the fix into CVS.  Regardless, as I already stated, I
think this module provides useful functionality often expected by
programs: guaranteed blocking behavior from stdio.

Regards,

Derek
#ifndef BLOCKING_IO_H
# define BLOCKING_IO_H

/* Get va_list.  */
# include 
# include 

# define printf blocking_printf
# define fprintf blocking_fprintf
# define vprintf blocking_vprintf
# define vfprintf blocking_vfprintf
# undef putchar
# define putchar blocking_putchar
# undef putc
# define putc blocking_putc
# define fputc blocking_putc
# define puts blocking_puts
# define fputs blocking_fputs
# define fwrite blocking_fwrite
# define fflush blocking_fflush
# de

Re: CVS problem with ssh

[Derek Price]

Richard M. Stallman wrote:

>I wrote the code, checking the specs, and got it to compile correctly.
>I don't have any easy way to test it--Emacs is not well suited to that
>purpose.  Could you test it?
>  
>

You neglected to include a wrapper for at least fflush().

>  subst_fwrite (buffer, 1, out_count, stdout);
>
>  return out_count;
>  
>

You need to free BUFFER after writing it in all your *printf variants.

>int
>subst_fwrite (char *buffer, size_t size, size_t count, FILE *stream)
>  
>

POSIX says fwrite accepts a BUFFER of type `const void *', not `char *'.

>{
>  int count_left = count;
>  int desc = fileno (stream);
>
>  while (1)
>{
>  int written;
>  fd_set set;
>
>  written = fwrite (buffer, size, count_left, stream);
>  
>

According to POSIX, a short return from fwrite does not guarantee that a
portion of the following item was not written for items of SIZE > 1, so
you need to convert BUFFER to COUNT * SIZE items of size 1 and loop
around writes of that to avoid writing data twice.  I'm not sure of the
most appropriate way to deal with overflow in COUNT * SIZE here.

>  if (written == count_left)
>   break;
>  
>

If errno != EAGAIN here, the function should return the short item count
anyhow, without molesting errno.  For instance, POSIX states that fputc
(and thus fwrite) can return a short count with errno = ENOSPC for a
disk full error.  This wrapper should not continue to attempt writes in
such a case.

>  /* Wait for space to be available.  */
>  FD_ZERO (&set);
>  FD_SET (desc, &set);
>  select (desc + 1, NULL, &set, NULL, NULL);
>
>  buffer += written;
>  count_left -= written;
>}
>
>  return count;
>}
>  
>

Regards,

Derek



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

Re: Unable to install CVS Server on Windows OS

[Derek Price]

[EMAIL PROTECTED] wrote:

>So, My requirement is to install CVS Server one machine and CVS Client on
>other machine for Windows OS Network which configuration is given below:-
>
>Operating System: Microsoft Windows 2000 Professional (32-bit)
>  
>

The GNU CVS project does not run as a server by default on Windows.  If
you must use a Windows CVS server and GNU CVS, you need to install the
Cygwin UNIX tools and most likely follow the Cygwin SSHD HOWTO to set up
a Windows OpenSSH Server, then follow the CVS instructions for setting
up a CVS server for SSH access in the CVS manual.

Alternately, you might try the CVSNT project.  They forked from GNU CVS
some time ago, but implement a CVS-like Server on Windows NT and its
derivatives (2000, XP, ...).

Derek




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

Re: CVS problem with ssh

[Derek Price]

Richard M. Stallman wrote:

>Not exactly.  If a solution like this were to go into GLIBC, then it
>becomes much more likely that it can be shared with GNULIB, which means
>that the infrastructure for it is already in CVS and that supporting it
>won't rely exclusively on the extremely small and overworked CVS team. 
>
>If this is what it takes for you to fix the bug, I will do this.
>  
>

If you can get this done, great!  I think it is the most complete way to
fix this bug since it also handles future (currently unknown and
arguably broken) tools that also set shared file descriptors to a
non-blocking status, as well as making the
stdio-always-blocks-and-no-data-sent-will-be-lost functionality
available to other applications that may need it.

Regards,

Derek



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

Re: CVS problem with ssh

[Derek Price]

Larry Jones wrote:

>Richard M. Stallman writes:
>  
>
>>If this is what it takes for you to fix the bug, I will do this.
>>
>>
>
>Instead of spending large amounts of effort on half-assed workarounds,
>why don't you make a concerted effort to explain the problem to the
>OpenSSH developers and encourage them to fix the actual problem?
>

Though I agree that it appears, at the least, to be rude to change the
blocking status of a potentially shared file descriptor, I can also see
the argument from the SSH side that they wish to be able to expect the
behavior they request from said file descriptor.

Since there is no actual standard on the matter, a widely available set
of *_block stdio variants seems a reasonable way to satisfy everybody,
to me, and could be used in other tools which encounter similar problems.

>  As far
>as I can tell, no one has actually bothered to do that, which leads me
>to believe that it's not really a serious problem for anyone.
>  
>

This is possible, though, as I previously presented, I am fairly certain
the race condition still exists.  Even if no one is currently reporting
problems, it may crop up again as networks and platforms change, or
perhaps as the new code gets tested on new variants of the same.

Of course, you may be correct that it makes the most sense to wait until
we see more actual problem reports with the new code before spending
time fixing it, but then, as long as someone else submits a complete
patch (i.e. it isn't my time being spent writing it), I don't mind
applying it so much since I am fairly sure the problem is still present,
if currently dormant.

Cheers,

Derek



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

Re: CVS problem with ssh

[Derek Price]

Richard M. Stallman wrote:

>Why not have glibc incorporate the moral equivalent of the "unlocked"
>stdio routines (fwrite_unlocked, printf_unlocked, etc.), like
>fwrite_block?
>
>I am not sure what "unlocked" means, 
>

I'm referring to the non-thread-safe stdio variants exported by glibc
and supported as a GNULIB module.

>but any solution like this would
>only work on GNU.  It would be better to use an equally simple
>approach that will work on other systems also: to define simple
>functions inside CVS to replace fwrite, fprintf, etc.
>  
>

Not exactly.  If a solution like this were to go into GLIBC, then it
becomes much more likely that it can be shared with GNULIB, which means
that the infrastructure for it is already in CVS and that supporting it
won't rely exclusively on the extremely small and overworked CVS team. 
Even if it isn't shared with GNULIB, we can probably rely on the GLIBC
folks to handle portability issues and import a file or two from GLIBC
directly into the CVS tree.

>My suggestion is to write a function cvs_fprintf that uses vsnprintf
>and alloca so as to generate the entire message, and then calls fwrite
>in a suitable loop with select to output the message.  That should be
>simple enough.  How about it?
>
>I will write the function for you if you don't want to write it.
>It would give me an opportunity to restudy how varargs works.
>  
>

Again, I am less afraid of writing this than maintaining it, though I
feel I have the time to do neither just now.  The whole of stdio would
need replacement functions in CVS for this to work consistently: fwrite,
fflush, fprintf, printf, etc. - not just fwrite and not just fprintf. 
If anyone were to contribute such a complete patch, I would consider it
more seriously, still subject, of course, to further comment from the
other CVS developers.

To summarize, if such a patch were accepted into GNULIB, I would have no
problem importing the module into CVS.  Only accepted into GLIBC, I
would have to think about it some more and examine how easy it would be
to make the wrappers stand-alone.  Applied only to CVS, I remain
reluctant to accept the probable maintenance cost but may consider a
complete patch, pending further comment from the other CVS developers
and the results of tests on diverse platforms.

Regards,

Derek



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

Re: cvs can't deal with big file

[Derek Price]

이현철 wrote:

>-rwxr-xr-x   1 root root 116M Jun 30 14:21 cscope.out
>
>when I did "cvs checkout", it stalled forever.
>
>what's wrong?
>  
>

I'm not sure. Quite possibly the server wasn't actually stalled but
needed so much memory for the operation it was constantly swapping
virtual memory to and from the disk, which could make the server slow to
a crawl.

Use a process monitor on the server to see if that is happening. Whether
it is or it isn't, if you set and export CVS_SERVER_SLEEP to some number
of seconds on the server, you should be able to give yourself time to
connect a debugger and see where the program is stuck. Let us know what
you discover!

Cheers,

Derek



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

Re: CVS problem with ssh

[Derek Price]

Why not have glibc incorporate the moral equivalent of the "unlocked"
stdio routines (fwrite_unlocked, printf_unlocked, etc.), like
fwrite_block?  Similarly for printf_block, fprintf_block, etc.  Most of
the stdio code would not need to be reimplemented since it is already
present in glibc and applications which wanted to rely on the blocking
behavior of stdout and stderr could use these routines instead of the
usual stdio?  In fact then they (and CVS) could probably get it down to
some #defines in a header without changing the existing stdio calls.

Regards,

Derek



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

Re: CVS problem with ssh

[Derek Price]

Richard M. Stallman wrote:

>Again, we could install loops around fwrite in handle_m, handle_e,
>handle_mbinary, and handle_mt and transfer most of the data coming
>across from the server correctly to stdout/stderr,
>
>Why do you say "most"?  I think we have already established that _all_
>the data will be transferred correctly.  Isn't that so?
>
>  but this still
>wouldn't handle other client output.
>
>Could you describe a case in which there would be a problem?  It would
>have to be a case in which substantial data is forwarded from the
>server, but the client also generates substantial data.
>  
>

The client rarely generates substantial data.  The case I am considering
is when the server dumps a substantial amount of data, followed by any
length printf from the client.  The dump of data from the server could
conceivably fill the stdio buffer without triggering a flush.  Any
printf following that would lose data if the attempted flush returned
EAGAIN.

Regards,

Derek



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

Re: CVS problem with ssh

[Derek Price]

Richard M. Stallman wrote:

>  Currently CVS does a single select on the stdout fileno
>before dumping as much data as it last received from the network to
>stdout via fwrite.  There is *no looping or checking for EAGAIN
>returns*, which I believe you advocated but, I am informed by Larry
>Jones and Paul Eggert, would likely be unportable to non-GNU systems.
>
>What do you mean by "unportable"?  As far as I can see, that change
>could not do any harm on any system.  If it fixes the problem on GNU
>systems, that's an important improvement, so why not do it?
>  
>

It might not do any additional harm.  I won't claim to understand the
issue completely, but I was told that there might be data loss since
EAGAIN may not mean what it is supposed to on some systems.  I suppose
that we would be no worse off than we are now.

Paul Eggert was also claiming that a successful select on STDOUT_FILENO
wouldn't necessarily imply a successful write of even a single byte to
the stdout stream (in a loop context, so I don't believe he was
referring to any potential race condition), though I know of no set of
conditions where that would happen.

>It sounds like you can rely on stdio on a GNU system.  
>

In the loop we are avoiding, perhaps.  In its current context, obviously
not, which is where I wanted to be able to rely on it.

>If other
>systems don't handle this reliably, maybe that is their fault.  You
>could report bugs in fwrite if they fail to write the data and don't
>return EAGAIN.
>  
>

Again, we could install loops around fwrite in handle_m, handle_e,
handle_mbinary, and handle_mt and transfer most of the data coming
across from the server correctly to stdout/stderr, but this still
wouldn't handle other client output.  That would require a complete
stdio wrapper, replacing all our calls to printf, fprintf, fflush, etc. 
This would be serious work and not easily maintainable.  It really seems
the much simpler solution for ssh to play nice with the standard file
descriptors.

Regards,

Derek



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

Re: Terminated with fatal signal 11

[Derek Price]

More modern systems sometimes leave a file named core.PID, where PID is
a number.  There may be other variations.  I usually search for
`*core*'.  Some systems need the command `ulimit -c unlimited' run
before the program which dumps core, though probably the default may be
changed to unlimited via some system pref parameter somewhere.

A CVS server would most likely be leaving the cores under a directory
under /tmp named `cvs-servXX', where XX is a random combination
of letters and numbers.  Normally CVS will remove the server tmp
directory, but it isn't supposed to after a core dump.

I can't do a whole lot without a core dump.  If none of the above work
for you, it is also possible to set "CVS_SERVER_SLEEP" to some number of
seconds on the server (most likely in inetd, xinetd, or a cvs server
wrapper script), then quickly connect to the server process using a
debugger while it sleeps, after starting it via Tortoise or whatever.

Good luck,

Derek


Gary Wake wrote:

> Love to but we don't have a 'core' file anywhere on the system
>
> Derek Price wrote:
>
>> Gary Wake wrote:
>>
>>  
>>
>>> Terminated with fatal signal 11
>>>
>>> Error, CVS operation failed
>>>   
>>
>>
>>
>> Can you find the server core dump and get us a stack trace from the
>> debugger?
>>
>> Regards,
>>
>> Derek
>>
>>
>>
>> ___
>> Bug-cvs mailing list
>> Bug-cvs@gnu.org
>> http://lists.gnu.org/mailman/listinfo/bug-cvs
>>  
>>
>
> ---
> This email message and any attachment(s) is intended only for the
> person(s) or entity(entities) to whom it is addressed. The information
> it contains may be classified as IN CONFIDENCE and may be
> legally privileged. If you are not the intended recipient any use,
> disclosure or copying of the message or attachment(s) is strictly
> prohibited. If you have received this message in error please notify
> us immediately and destroy it and any attachment(s).
> Thank you. The Ministry of Social Development accepts no
> responsibility for changes made to this message or to any
> attachment(s) after transmission from the Ministry.
> ---
>




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

Re: CVS problem with ssh

[Derek Price]

Richard M. Stallman wrote:

>The code currently works by making the assumption that network reads
>will always return chunks of data small enough to be written to stdout
>in a single call to fwrite after a single select on fileno (stdout).
>
>What would make some data too long?  The nmemb argument of fwrite is a
>size_t, and unless fwrite has a bug, it should handle whatever amount
>of data you specify.
>
>I don't see why a large amount of data would cause a failure.
>Previous messages claimed that it could, but I think I've refuted that
>claim.  Is there a flaw in the argument I gave?
>  
>

I've simply been describing what I believe is the current race
condition.  Perhaps you misunderstood the status of the current
workaround in CVS (you may be thinking "fix", but I am attempting to
refute that)?  Currently CVS does a single select on the stdout fileno
before dumping as much data as it last received from the network to
stdout via fwrite.  There is *no looping or checking for EAGAIN
returns*, which I believe you advocated but, I am informed by Larry
Jones and Paul Eggert, would likely be unportable to non-GNU systems.

Currently, this workaround appears to prevent the original problem, but
I think it is working because the select usually implies there is more
room for writing to stdout than we have data only because reading from
the network is slower than writing to disk.  The whole point of this
discussion, I thought, was that we could rely on an EAGAIN return from
GNU fwrite and could retry the write with the remaining data when there
was no room?

Since there is no looping going on and any single fwrite of an arbitrary
amount of data > 1 byte should be able to sometimes trigger the EAGAIN
even after a select, I suspect the race condition described above
exists.  On a fast network with a slow disk, the problem should still be
reproducible.  Perhaps this could be verified by piping the CVS stdout &
stderr to netcat across an even slower network or somesuch.  I don't
know and I don't really have the facilities here to test it.


Though it was not what I was arguing in the above thread - I only meant
to point out that, as I understand the problem, the current situation is
not a true fix, I will reiterate that I still agree with Larry and am
loathe to reimplement stdio in CVS.  I would much prefer to be able to
rely on stdio for a situation like this.

I most certainly don't have the time to reimplement it myself at the
moment and even were a complete patch submitted I would still be
reluctant to accept it due to potential maintenance hassles.  I
understand a similar stdio reimplementation happened in Emacs in the
past and the team eventually gave up on it due to unmaintainability.

Cheers,

Derek



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

Re: Terminated with fatal signal 11

[Derek Price]

Gary Wake wrote:

> Terminated with fatal signal 11
>
> Error, CVS operation failed


Can you find the server core dump and get us a stack trace from the
debugger?

Regards,

Derek



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

Re: CVS problem with ssh

[Derek Price]

Richard M. Stallman wrote:

>Regardless of the fact that this will usually hold true, the algorithm
>still contains a race condition, might not hold true across diverse
>platforms, and is not a true fix.
>
>I do not see the race condition.
>  
>

The code currently works by making the assumption that network reads
will always return chunks of data small enough to be written to stdout
in a single call to fwrite after a single select on fileno (stdout).  As
long as network reads are always slower that writes to stdout, this
should hold true, but it does not sound like an assumption which can be
counted on to prove consistently valid across all platforms and
circumstances.  At the least there is the case where the stdout output
is being piped across an even slower network.

Regards,

Derek



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

Re: CVS problem with ssh

[Derek Price]

Richard M. Stallman wrote:

>It's possible that this fix is a full and correct solution.  If the
>descriptor has room for at least one byte, `write' won't return
>EAGAIN; it will write at least one byte.  It may write less than the
>whole buffer that was specified.  If the stdio code handles that case
>correctly, the results should be correct.
>

Are you telling me that the glibc implementation of streams provides a
buffer with infinite space?  POSIX 1003.1 says fwrite() returns errors
as fputc().  POSIX 1003.1 says about fputc:

The fputc() function shall fail if either the STREAM is unbuffered
or the STREAM's buffer needs to be flushed, and:

[EAGAIN]

The O_NONBLOCK flag is set for the file descriptor underlying
/stream/ and the thread would be delayed in the write operation.



>Roland, does GNU libc
>handle that case correctly?  I expect it does, since that case can
>arise in many situations.  If it were not handled correctly, we would
>hear lots of screaming--I think.
>  
>

I bumped CVS's test for this error up to dumping about 110MB of data to
stdout, and it still passes, but not for the reason you think, I think. 
I'm watching the file fill more slowly than it should if limited only by
the disk, and the data is coming from both the network and diff.  I'm
guessing that the file writes are simply usually faster than the reads
of data produced at a high CPU cost and transferred across a network. 
Regardless of the fact that this will usually hold true, the algorithm
still contains a race condition, might not hold true across diverse
platforms, and is not a true fix.

Regards,

Derek



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

Re: CVS problem with ssh

[Derek Price]

Richard M. Stallman wrote:

>Couldn't this be fixed easily by changing cvs to handle EAGAIN
>in a simple way when outputting to stdout?  If it just sleeps for a second
>and tries again, that ought to fix the problem.
>  
>

Mark Baushke installed Frank Hemer's partial work-around for this
problem some time ago.  This should have been released with CVS 1.11.18
& 1.12.10:

2004-08-17  Mark D. Baushke  <[EMAIL PROTECTED]>

* sanity.sh (sshstdio): Fix comment typo plus gratuitous
reformatting.

* client.c (handle_m): Workaround to deal with stdio getting put
into non-blocking via redirection of stderr and interaction with
ssh on some platforms. On those boxes, stdio can put stdout
unexpectedly into non-blocking mode which may lead to fwrite() or
fflush() failing with EAGAIN, but cvs not checking for the error.
(Patch suggested by Frank Hemer <[EMAIL PROTECTED]>.)

* client.c (handle_e): Similar fix for stderr.
* sanity.sh (sshstdio): New test for non-blocking stdio via ssh.


This fix selects on the file descriptor associated with the stream
before writing to the stream via stdio, which is probably wrong on
several levels, as Paul Eggert pointed out in this discussion:
. 
This is also definitely only a workaround and I suspect won't work for
very large file sizes since the select only guarantees that the
descriptor is ready for a single byte, not an arbitrary amount of data,
though it appears to handle the original test case.

I agree with Larry Jones, that the problem is deeper.  It seems to me
that any program writing to stderr or stdout via stdio should have the
reasonable expectation that the stream will remain blocking, or perhaps
remain non-blocking, as the case may be.

If we have to fix anything on the CVS side, I think the only complete
fix (read working in all cases) would be to replace all our stdio calls
with our own buffering writes that handled things regardless of EAGAIN. 
Much of the code that would be necessary is already in CVS 1.12.x - it's
used for the network buffers on the server side, but nobody has
submitted the patch to apply it everywhere and, as I said, I agree now
that this change would be wrong so I have not written it myself.  The
same would need to be done in any program piping output through ssh,
some of which don't have CVS's infrastructure, and this seems unreasonable.

Regards,

Derek



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

Re: Core dump during CVS Commit

[Derek Price]

Smruti wrote:

>Hi Dreek,
>
>Thanks for the mail.
>Could you update me how shall I take stack trace from core?
>  
>

Using the GNU debugger (gdb), the command is `bt' (for "backtrace"). 
Thus, assuming that you were using /usr/bin/cvs and that your core dump
created a file named "core.1234" (where "1234" couild be any process id):

$ gdb /usr/bin/cvs core.1234
...assorted gdb bootup output...
(gdb) bt
...SEND THIS OUTPUT...
(gdb)


The commands may be slightly different if you need to use the Solaris
debugger.

The core dump file should have been created in whatever directory you
ran the offending CVS command from.  If it wasn't, you might have to run
the command `ulimit -c unlimited' to enable core dumps.  At least,
that's the command on Linux - I'm not sure that is the same on Solaris
either.

Regards,

Derek



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

Re: OpenBSD 3.6 sparc64 Build Failure on TRUNK

[Derek Price]

Alexander Taler wrote:

>  Derek> One of the things it does is verify that the requested executable
>  Derek> exists and that it is a GNU application.  This is what caught us - the
>  Derek> script was assuming that any GNU program is good and therefore that
>  Derek> its failure meant a real failure.  I added a new bison wrapper based
>  Derek> on missing to handle the version check.  Let me know if it doesn't
>  Derek> work for you.
>
>Should I let you know if it does?  Because it does.
>Thanks.
>  
>

Great.  Thanks.  :)

Derek



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

Re: Core dump during CVS Commit

[Derek Price]

Smruti wrote:

>Hi,
>
>I have insttaled CVS 1.11.19 on Solaris 5.8 and have created repository.
>CVS is running as Password Auth server and have made necessary changes in
>inetd.conf to include follwoing
>cvspserver  stream  tcp nowait  cvs /usr/local/bin/cvs
>oot -f  --allow-root=/usr/local/cvsroot pserver
>and changed "/etc/services" to include cvspserver  entry.
>
>Now whenever I try to import anything in CVS to create the project, it
>always gives me "Segementation Fault" or "cvs import aborted due to abort
>signal"
>
>FYI:  CVSROOT is set to =/export/home/smruti/temp/cvs
>$ cvs -tn import test_cvs test start
> -> main loop with CVSROOT=/export/home/smruti/temp/cvs
>cvs [import aborted]: received abort signal
> -> Lock_Cleanup()
>
>Can anyone help me? Let me know if any other info is required.
>  
>

Can you provide a stack trace from the core?

Derek



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

Re: OpenBSD 3.6 sparc64 Build Failure on TRUNK

[Derek Price]

Alexander Taler wrote:

> case "$1" in
> --run)
>   # Try to run requested program, and just exit if it succeeds.
>   run=
>   shift
>   "$@" && exit 0
>
> Where do you suppose that check should be?


Actually, `missing' only `exit 0's when the program exited with a
success code.  Otherwise it will fall through and perform various
actions depending on the argument to --run.

One of the things it does is verify that the requested executable exists
and that it is a GNU application.  This is what caught us - the script
was assuming that any GNU program is good and therefore that its failure
meant a real failure.  I added a new bison wrapper based on missing to
handle the version check.  Let me know if it doesn't work for you.

This problem should go away when we start using Automake 1.10 since it
won't allow Yacc targets to be built without the
--enable-maintainer-mode argument to configure.

Regards,

Derek



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

Re: OpenBSD 3.6 sparc64 Build Failure on TRUNK

[Derek Price]

Alexander Taler wrote:

> $ make
> make  all-recursive
> Making all in lib
> cp ./alloca_.h alloca.h-t
> mv alloca.h-t alloca.h
> /bin/sh ../build-aux/ylwrap getdate.y y.tab.c getdate.c y.tab.h
> getdate.h y.output getdate.output -- /bin/sh
> /home/dissent/prj/libcvs/var/ccvs-1.12/build-aux/missing --run bison -y
> /home/dissent/prj/libcvs/var/ccvs-1.12/lib/getdate.y:202:
> unrecognized: %parse-param
> /home/dissent/prj/libcvs/var/ccvs-1.12/lib/getdate.y:202:Skipping
> to next %
> /home/dissent/prj/libcvs/var/ccvs-1.12/lib/getdate.y:203:
> unrecognized: %lex-param
> /home/dissent/prj/libcvs/var/ccvs-1.12/lib/getdate.y:203:Skipping
> to next %
> *** Error code 1


There are a few issues here:

   1. getdate.y from GNULIB requires at least Bison 1.875.
   2. getdate.y shouldn't be recompiled unless it has changed, but
  sometimes the timestamps can get munged when checking out of CVS.
   3. When either of the above two problems are not avoided, the
  build-aux/missing script is supposed to just update the
  timestamps, print a warning message, and exit with a success
  code.  This may not happen if your lib/getdate.c file is actually
  missing, for instance if you ran a `make maintainerclean'.


If your `missing' script is failing for any reason other than a bad
bison AND a missing lib/getdate.c, then I would like to get to the
bottom of it.  Otherwise, your solution should be to update to a more
recent Bison (which we assume for anyone willing to run `make
maintainerclean'), or pull the current lib/getdate.c from CVS and avoid
the rebuild.

I have documented the Bison 1.875 requirement in HACKING.

Regards,

Derek



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

Re: CVS update: /ccvs/lib/

[Derek Price]

Conrad T. Pino wrote:

>I added "lib/lstat.c" because linker didn't have definition for external
>symbol "_rpl_lstat" and a grep found only 1 definition.  I had seen this
>commit to "windows-NT/config.h" files:
>https://ccvs.cvshome.org/servlets/ReadMsg?list=cvs&msgNo=3881
>which I assume meant the reference to "rpl_lstat" was intentional.
>
>I've always assumed commits to "windows-NT" config files reflected a well
>considered plan so I often don't examine the committed changes other than
>noting the file names.  I now see room for process improvement.  What are
>your thoughts?
>  
>

Well-considered, perhaps, but without a test platform it can be
challenging to verify that I did not make any mistakes.  In this case, I
thought I knew what I was doing and was just trying to save you some
work.  Apparently I neglected to consider that the #include "lstat.h"
was going to override my windows-NT/config.h changes in this case.

Sounds like the `#define lstat stat' can be removed from
windows-NT/config.h.in.footer in conjunction with the other changes we
discussed.

Please feel free to review any committed changes that you see fit to and
discuss them on bug-cvs.  :)

Regards,

Derek



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

Re: Conditional/Comment Error? lib/canonicalize.c

[Derek Price]

Conrad T. Pino wrote:

>Can someone suggest which one of the following 2 changes (if any)
>is the best one to commit?
>  
>

Neither.  Apparently we have been breaking the GNU commenting
conventions for #ifdefs and GNULIB has it right:
, though they
left off the comment on the #else, if you would like to submit a patch
to bug-gnulib.

Regards,

Derek



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

Re: Windows Build - lib/canonicalize.c & lib/filenamecat.h

[Derek Price]

Conrad T. Pino wrote:

>Conditional compilation on Windows compiles code that doesn't
>use the "file_name_concat" function which lead me to omit the
>file "lib/filenamecat.c" from the "libcvs" project.  However
>the '#include "filenamecat.h"' in "lib/canonicalize.c" is
>unconditional triggering VC6 to create a false dependency.
>
>Does surrounding the '#include ...' in "lib/canonicalize.c"
>with the same conditional compilation logic as found later on
>in the same file seem useful to the project as a whole?
>  
>

I would hazard that that should be fine, though the change should be
submitted to GNULIB.

Regards,

Derek



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

Re: CVS update: /ccvs/lib/

[Derek Price]

[EMAIL PROTECTED] wrote:

>Log:
> * libcvs.dsp: Add files "filenamecat.h" and "lstat.c" to project.
>  
>

Your change to filenamecat.h is correct - that's a rename of the
path-concat module from GNULIB, but I don't think you want lib/lstat.h &
lib/lstat.c on Windows.  lib/lstat.c contains a wrapper for lstat that
works around a very specifc lstat bug on some UNIX systems & lib/lstat.h
is going to override the definition of lstat from windows/config.h. 
Perhaps leaving out lib/lstat.c & lib/lstat.h and adding a
windows-NT/lstat.h like the following would be a more complete solution?

#ifndef LSTAT_H
# define LSTAT_H
# include 
# define lstat stat
#endif


I think window-NT/filesubr.h will need to include "lstat.h" after this
change as well.

By the way, while I am thinking about it, re our earlier discussions of
Cygwin and Windows replacement functions, Cygwin settled on using .lnk
files (Windows shortcuts) as the Windows representation of UNIX
symlinks.  It might make sense for CVS on Windows to use stat & lstat
wrappers that behaved similarly.  i.e. stat would follow .lnk references
until they resolved to something real or failed to (check POSIX but I
think this would mean an error) and an lstat that returned the actual
stat information from the .lnk file.  Of course, this would probably
also require wrappers for "open" and "fopen" that also followed
shortcuts as if they were symlinks.

At the very least, this would make it much easier for an admin to set up
symlinks in a Windows CVS repository.  They could just create shortcuts
from Windows Explorer.

Regards,

Derek



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

Re: patch: implement %{t} for loginfo

[Derek Price]

Todd Vierling wrote:

>NetBSD, among other projects, has used a change similar to this for many
>years, to allow the tag name to be passed to loginfo scripts.  This allows
>the popular log_accum script to supply the tag name in the Subject line.
>
>Below is a diff against 1.12.12, and is really very trivial; I wonder if
>it's just oversight that prevented this from being contributed sooner.  It
>would be really nice if this didn't require external maintenance anymore.  :)
>  
>

It was an oversight, basically.  I've committed your change plus a
parallel change in taginfo, except I used a capital `T' rather than
`t'.  `t' was already in use in the taginfo file as the tag name being
added/moved/deleted.

I also added documentation and tests.

Regards,

Derek



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

Re: GZip level restriction

[Derek Price]

Derek Price wrote:

>Derek Price wrote:
>  
>
>>>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.
>>
>>
>
>
>Hey Larry,
>
>Can I assume that no comment for almost two weeks means you have no
>further objections to this patch going into feature?
>  
>

Since I haven't heard any further objections to this change in several
days, I have committed it to feature.

Cheers,

Derek



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

Re: GZip level restriction

[Derek Price]

Derek Price wrote:

> >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.


Hey Larry,

Can I assume that no comment for almost two weeks means you have no
further objections to this patch going into feature?

Cheers,

Derek




___
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:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>>   1. Corrects an incorrect check for a successful return from
>>  getlogin_r to assume only 0 means success, per the POSIX2 spec:
>>  
>> <http://www.opengroup.org/onlinepubs/009695399/functions/getlogin_r.html>.
>>   2. Moves the check for GLOB_MARK directory status (and the append of
>>  `/') into glob_in_dir, where it is more efficient than performing
>>  a second pass and sometimes calling stat a second time on each
>>  file or directory.  All calls to stat are avoided when
>>  dirent->d_type is available.  No call to realloc of the directory
>>  name is ever necessary since room for the slash can be allocated
>>  in the first pass.
>>
>>
>
>These changes sound reasonable, though we should submit them as
>separate patches.
>  
>

The attached  glibc-understand-getlogin_r.diff may come off as a little
fuzzy, but it should work for (1).  I've also reattached the patch for 2
& 3 minus the above chunk, as glibc-glob-list-links2.diff.

>Is (2) independent of (3)?  (Please see below for why this is important.)
>  
>

(2) could be separated with a little more work, but lets finish the
discussion below before I provide a second patch.

>>   3. Ignores broken links only when GLOB_ONLYDIR is set.  With glibc
>>  versions 2.3.3 through 2.3.5, the following in an empty directory
>>  would return nothing:
>>
>>  ln -s doesnt-exist linkname
>>  glob ("*", ...)
>>
>>  This fix syncs with the comments in the file, syncs with the
>>  POSIX2 spec, restores the pre-glibc-2.3.3 behavior, and simply
>>  makes more sense - why should `ls *' fail to list broken links?
>>
>>
>
>This change sounds controversial to me.  glibc 2.3.5 behaves similarly
>  
>

It may be.  It looks like the change was intentional
(http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/sysdeps/generic/glob.c?rev=1.52&content-type=text/x-cvsweb-markup&cvsroot=glibc),
but I still disagree.

>to Solaris 8 and to Solaris 10 -- I just checked, with the following
>program and with the working directory containing only a dangling
>symlink:
>
>  #include 
>  #include 
>
>  int
>  main (void)
>  {
>glob_t g;
>int r = glob ("*", 0, NULL, &g);
>int i;
>
>if (r != 0)
>  {
>fprintf (stderr, "glob failed (%s)\n",
> r == GLOB_NOMATCH ? "GLOB_NOMATCH"
> : r == GLOB_NOSPACE ? "GLOB_NOSPACE"
> : "other glob failure");
>return 1;
>  }
>
>for (i = 0; i < g.gl_pathc; i++)
>  puts (g.gl_pathv[i]);
>return 0;
>  }
>
>Solaris 8 and 10 both report "glob failed (GLOB_NOMATCH)".
>
>Let's separate (3) into a separate patch and think about it more
>carefully before submitting it.
>  
>

This would involve creating at least one new function for (2) which
would just need to be removed again for (3), and the POSIX2 glob spec
states that glob returns GLOB_NOMATCH only when, "the pattern does not
match any existing pathname, and GLOB_NOCHECK was not set in flags." 
(http://www.opengroup.org/onlinepubs/009695399/functions/glob.html)

Nowhere does the POSIX2 glob spec specify that a broken symlink should
not be considered an "existing pathname".  After all, the link exists,
only its target does not.  Interpreted in this way, glob cannot be used
by a program which, for instance, wished to verify that all links
matching a pattern had valid targets since the broken links would not be
returned by glob.

If glob is only going to consider a link as if it is its target, then
what about the case where a matching link points to a file in the same
directory that also matches the pattern?  Should glob only return one or
the other?

Perhaps a GNU extension similar to GLOB_ONLYDIR is in order
(GLOB_VERIFY_SYMLINKS?), but I do not think glob should be making these
value judgements when the user did not request it.  It certainly does
not appear to be implied by the POSIX spec as I read it.

>Have you investigated with FreeBSD glob does?  
>

No, but I ran your test program on NetBSD 1.6.1 and it does return the
broken symlink.

I copied and pasted most of my above commentary on why this should be
considered a bug into
<https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=126460>, where I
found the discussion that apparently caused the original "bug fix" that
broke this.

Cheers,

Derek

--- ../glob-glibc2gnulib/lib/glob.c 2005-05-31 18:12:01.0 -0400
+++ lib/glob.c  2005-05-27 13:52:28

Re: CVS update: /ccvs/windows-NT/sys/

[Derek Price]

Conrad T. Pino wrote:

>Hi Derek,
>
>  
>
>>From: Derek Price
>>
>>I think renaming this file "sys_types.h", starting it with `#include
>>', then defining what else you need would be an easier
>>interface to maintain.
>>
>>
>
>Are you implying changing all #include  in CVS? I assume no.
>  
>

Well, it may be an option if necessary, similar things have been
proposed for GNULIB modules, but none has turned out to be necessary yet.

In this case I was thinking that the Windows config.h.in.footer could
#include "sys_types.h" and get the effect you are looking for.

>I intend the "windows-NT" version be used in all locations cited at the
>end of this message.
>
>My first try was your suggestion with the current name only to discover VC6
>reopens the "windows-NT" file because "windows-NT" is in the include path.
>  
>

Which is why I proposed renaming the Windows include to "sys_types.h". 
I also dislike polluting the windows-NT source directory space with a
directory structure mimicing a standard C89/POSIX include structure with
no differentiation from the other source files if you can avoid it.  If
you must, I might prefer moving your sys directory under a
windows-NT/include directory and making the other changes you needed to
to accomodate that, but I still prefer avoiding the sys directory
entirely in favor of the "sys_types.h" file.

Regards,

Derek



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

Re: windows-NT/pwd.c - struct passwd - Home Directory

[Derek Price]

Conrad T. Pino wrote:

>My ideas for the API are UNIX/POSIX specifications are non-negotiable and
>internal APIs below that are discardable upon discovery of the better way.
>  
>

Basically.

In theory, you could take up desired changes with the POSIX spec with
IEEE.  Of course, if you did get some changes through, it would likely
be years before we could support them fully in CVS and GNULIB - that
doesn't happen until it is determined that hosts which require older
(i.e. POSIX2) specifications are no longer reasonable porting targets. 
As an example, GNULIB and CVS both still support ANSI C89
specifications.  I would not hazard a guess as to when C99 will be
judged a reasonable assumption.  GNULIB just finally dropped support for
SunOS 4.1.4, about a year and a half after Sun did.  Of course, the
official policy of GNULIB is to drop support when the provider of the OS
does, but it isn't always noticed right away.

Also, for a traget like Windows, compliance may be "as close as
possible" to the POSIX spec.  Sometimes a POSIX spec will make reference
to other POSIX specs, and if, for instance, the getpwuid spec referenced
the /etc/passwd file directly, we might only be able to approximate the
desired result.  What you and I are debating, I think, is what is
morally closest to the POSIX spec on Windows.


As an aside, it might sometimes be interesting to look at what Cygwin
did in some of these cases.  Their code is GPL'd.  Of course, I've
looked at it before, and it tends to be much too involved to import into
CVS easily - they have their own userid system overlaid on top of
Windows, for instance, which would likely make getpwuid a challenge to
import, and similar things happen with their file descriptor
implementation, but something like their implementation of usleep() or
nanosleep() might not have so many internal dependencies.

Cheers,

Derek



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

Re: CVS update: /ccvs/windows-NT/

[Derek Price]

Conrad T. Pino wrote:

>The timer code in "lib/nanosleep.c" looks very good but I'm uncomfortable
>making a commitment to making the change because:
>
>* I don't understand UNIX style signal behavior well.
>  
>

Basically, signals normally cause a process to exit unless a handler is
installed.

>* Millisecond resolution delays without surrendering the time slice are
>  possible by creating an unsignaled Event then waiting for that Event
>  with a time out value.  This feature requires hardware support in CPU
>  otherwise behavior reverts to the PC BIOS rate.
>
>I'll work up a test program to measure "select" time out granularity in
>the next few days.
>
>I'll support you to the best I can should you choose to make the change.
>  
>

This was just a suggestion as I thought it might simplify things.  You
can make the call.

I would argue that the signal handling is no argument since
windows-NT/run.c is making calls to signal, I would assume that the API
is likely similar to UNIX and whoever wrote lib/nanosleep.c knew what
they were doing until we discover problems.

Your timer granularity argument sounds reasonable, however.  If you need
to reimplement usleep anyhow, then you can't really compuile
lib/nanosleep.c on Windows without rewriting it anyhow.  I found this
thread where we discussed this previously:
.  Jim
Hyslop suggests that NTP (www.ntp.org) uses QueryPerformanceCounter()
and QueryPerformanceFrequency() to hack around the problem with about a
300ns granularity.

>Can you share some ideas on what I need to do to get "sanity.sh" to run
>on Windows 2000?
>  
>

I have had it up and running before using a version of CVS built with
cygwin (http://cygwin.com), using the bash and other tools from cygwin. 
Most of the hacks I made to get that to work should still be in sanity.sh.

IIRC, however, testing an executable built with MSVC is a different
story because it can sometimes use "\" and sometimes "/" as a file
separator.  No one has ever installed the machinery in sanity.sh to
handle that.  It might be possible to make a small change in
dotest_internal() that seds the program output and replaces all
backslashes with forward slashes before the text goes to expr for the
comparison.  That is where I think I would start trying to get this running.

>Does the MKS Tool Kit "sh" implementation help any?
>
>Is Windows 2000 "bash" combined with MKS Took Kit a better start?
>  
>

Like I said, I started with Cygwin and the Cygwin bash.  Don't know what
the differences are between MKS and Cygwin.  sanity.sh requires a
working /bin/sh and a few standards compatible tools: expr, id, tr, and
sed are the most sensitive tools, I believe.  If you are attached to MKS
but one of those tools is broken, you might get sanity.sh running by
building and installing the GNU versions there.  The TESTS file
describes where to get them.

>>>* filesubr.c: Modify "get_homedir" to use "woe32_home_dir".
>>>* pwd.c: Modify "getpwuid" to use "woe32_home_dir" & "woe32_shell".
>>>Append "USERNAME" to "login_strings" array.
>>>  
>>>
>>Again, unless USERNAME is a read-only env var, if there is a Windows API
>>for retrieving these sorts of values, it should probably be used in the
>>POSIX replacements in favor of writable environment variables.
>>
>>
>
>I agree in principle but several things indicate this is a non-issue:
>
>* Microsoft's workstation security practices commonly grant the CVS user
>  root equivalent privilege on the executing platform.  Why infringe on
>  the user's right to destroy local resources since they're effectively
>  all owned by the workstation user?
>  
>

One more time:

   1. I think a POSIX-compliant getpwuid function, or as near as we can
  get on Windows, should not use environment variables if at all
  possible if you wish to implement it in such a way that it
  complies as closely as possible with the POSIX spec so that it may
  be shared with other projects.
   2. I think there is no harm in the CVS get_homedir function using env
  variables.  CVS does that on UNIX on exactly the principle you
  state.  If the user set it then we will presume they knew what
  they were doing and let system security measures take care of
  things if the user is asking us to do something we shouldn't.


>* The better security route indicates we should remove "login_strings"
>  altogether since the addition/removal of a single variable doesn't
>  close the security vulnerability.
>  
>

This is exactly what I was advocating.  To meet the POSIX specs, these
functions should try to obtain the login name securely.  The POSIX
argument stands regardless, but a get_homedir-like wrapper would be
wrong here for CVS as well.  This value is entered in the repository as
the author of a commit and I would rather know that the user had to have
a password for the userid being used.  At the least, it shouldn't be so
easy to forge the 

Re: OpenBSD 3.6 sparc64 stable crash

[Derek Price]

Alexander Taler wrote:

>+2005-05-31  Alexander Taler <[EMAIL PROTECTED]>
>+
>+  * rcscmds.c: Change type of call_diff_argc_allocated from int to
>+  size_t, to match the prototype of run_add_arg_p().  This fixes a
>+  bus error in OpenBSD 3.6 sparc64.
>  
>

Makes sense.  int & size_t probably have different sizes on this 64 bit
platform, which could explain the bus error.  Thanks for the patch! 
I've committed it on stable and feature.

Regards,

Derek




___
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:

>Derek Price <[EMAIL PROTECTED]> writes:
>  
>
>>submission part.  Perhaps it would be smoother if someone already known
>>to the glibc team introduced me and this patch?
>>
>>
>
>Yes, probably.  I'm willing to have a go at it.
>
>I suggest submitting two patches.
>
>(1) the part that makes it work with gnulib.
>  
>

glob-glibc2gnulib11.diff attached:

2005-05-31  Derek Price  <[EMAIL PROTECTED]>
Paul Eggert  <[EMAIL PROTECTED]>

* glob.c:  Update copyright.  Assume freestanding C89 compiler.
Simplify cruft that may be replaced with GNULIB modules.  Make no
attempt to find 64-bit versions of file access functions
directly when
!_LIBC.  Move definitions of GLOB_* macros to glob_.h.
(DIRENT_MUST_BE, DIRENT_MIGHT_BE_SYMLINK, DIRENT_MIGHT_BE_DIR): New
macros to abstract dirent->d_type access.
(GETPW_R_SIZE_MAX, LOGIN_NAME_MAX): New macros to abstract sysconf
access.
* glob.h: Protect/define contents as necessary to coexist with
GNULIB.


>(2) the bug fix.
>
>It's OK for (2) to assume that (1) has already been applied.
>  
>

glibc-glob-list-links.diff attached.  Despite its name, it actually does
three things:

   1. Corrects an incorrect check for a successful return from
  getlogin_r to assume only 0 means success, per the POSIX2 spec:
  <http://www.opengroup.org/onlinepubs/009695399/functions/getlogin_r.html>.
   2. Moves the check for GLOB_MARK directory status (and the append of
  `/') into glob_in_dir, where it is more efficient than performing
  a second pass and sometimes calling stat a second time on each
  file or directory.  All calls to stat are avoided when
  dirent->d_type is available.  No call to realloc of the directory
  name is ever necessary since room for the slash can be allocated
  in the first pass.
   3. Ignores broken links only when GLOB_ONLYDIR is set.  With glibc
  versions 2.3.3 through 2.3.5, the following in an empty directory
  would return nothing:

  ln -s doesnt-exist linkname
  glob ("*", ...)

  This fix syncs with the comments in the file, syncs with the
  POSIX2 spec, restores the pre-glibc-2.3.3 behavior, and simply
  makes more sense - why should `ls *' fail to list broken links?


2005-05-31  Derek R. Price  <[EMAIL PROTECTED]>

* glob.c: #include 
(glob): Only 0 return from getlogin_r means success, according
to POSIX
2.  Move GLOB_MARK rescan into...
(glob_in_dir): ...here - it improves efficiency.  Don't fail to
return
broken links when GLOB_ONLYDIR is not set.
(link_exists_p): Rename to...
(is_dir_p): ...this and update functionality accordingly.



Regards,

Derek
--- ../glibc-2.3.5/sysdeps/generic/glob.c   2004-10-27 14:21:02.0 
-0400
+++ lib/glob.c  2005-05-31 18:12:01.0 -0400
@@ -1,4 +1,4 @@
-/* Copyright (C) 1991-2002, 2003, 2004 Free Software Foundation, Inc.
+/* Copyright (C) 1991-2002, 2003, 2004, 2005 Free Software Foundation, Inc.
This file is part of the GNU C Library.
 
The GNU C Library is free software; you can redistribute it and/or
@@ -16,23 +16,16 @@
Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA.  */
 
-/* AIX requires this to be the first thing in the file.  */
-#if defined _AIX && !defined __GNUC__
- #pragma alloca
-#endif
-
 #ifdef HAVE_CONFIG_H
 # include 
 #endif
 
-/* Enable GNU extensions in glob.h.  */
-#ifndef _GNU_SOURCE
-# define _GNU_SOURCE   1
-#endif
+#include 
 
 #include 
 #include 
 #include 
+#include 
 
 /* Outcomment the following line for production quality code.  */
 /* #define NDEBUG 1 */
@@ -40,30 +33,7 @@
 
 #include  /* Needed on stupid SunOS for assert.  */
 
-
-/* Comment out all this code if we are using the GNU C Library, and are not
-   actually compiling the library itself.  This code is part of the GNU C
-   Library, but also included in many other GNU distributions.  Compiling
-   and linking in this code is a waste when using the GNU C library
-   (especially if it is a shared library).  Rather than having every GNU
-   program understand `configure --with-gnu-libc' and omit the object files,
-   it is simpler to just do this in the source for each such file.  */
-
-#define GLOB_INTERFACE_VERSION 1
-#if !defined _LIBC && defined __GNU_LIBRARY__ && __GNU_LIBRARY__ > 1
-# include 
-# if _GNU_GLOB_INTERFACE_VERSION == GLOB_INTERFACE_VERSION
-#  define ELIDE_CODE
-# endif
-#endif
-
-#ifndef ELIDE_CODE
 #if !defined _LIBC || !defined GLOB_ONLY_P
-
-#if defined STDC_HEADERS || defined __GNU_LIBRARY__
-# include 
-#endif
-
 #if defined HAVE_UNISTD_H || defined _LIBC
 # include 
 # ifndef POSIX
@@ -73,22 +43,13 @@
 # endif
 #endif

Re: CVS update: /ccvs/windows-NT/

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

[EMAIL PROTECTED] wrote:

>Log:
> * unistd.c, unistd.c: Add new "usleep" function using "my_usleep"
> logic taken from "woe32.c" file.
> * woe32.c, woe32.h: Add new (woe32_home_dir,woe32_shell) functions.
> * woe32.c: Modify "woe32_nanosleep" to use "unistd.h" "usleep".


Glancing at lib/nanosleep.c, the usleep() implementation in
windows-NT/unistd.c, and based on the fact that signal() and select()
appear to be used elsewhere in windows-NT/*.c, it looks like
lib/nanosleep.c would compile on Windows with the right config.h.in.in
definitions.  Then the woe32_nanosleep and unistd.c usleep could be
removed in favor of the rpl_nanosleep in lib/nanosleep.c.

> * filesubr.c: Modify "get_homedir" to use "woe32_home_dir".
> * pwd.c: Modify "getpwuid" to use "woe32_home_dir" & "woe32_shell".
> Append "USERNAME" to "login_strings" array.


Again, unless USERNAME is a read-only env var, if there is a Windows API
for retrieving these sorts of values, it should probably be used in the
POSIX replacements in favor of writable environment variables.

Regards,

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

iD8DBQFCnLXVLD1OTBfyMaQRAqR1AJ9b7K8yExwUCc0EIj4jUrersoSyJQCgsjec
sHEymcNNNjRzXscCVdl36pY=
=1rqH
-END PGP SIGNATURE-




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

Re: CVS update: /ccvs/windows-NT/sys/

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

[EMAIL PROTECTED] wrote:

>#ifdef _MSC_VER
>
>#pragma once
>
>/* Compatable with Visual C++ 6.0 - cl.exe 12.00 */
>#if _MSC_VER != 1200
>#pragma message ( "Please email Microsoft's " )
>#pragma message ( "file to <[EMAIL PROTECTED]>" )
>#endif /* _MSC_VER != 1200 */
>
>/***/
>/* Mimic what Micrsoft defines in their  */


I think renaming this file "sys_types.h", starting it with `#include
', then defining what else you need would be an easier
interface to maintain.

Regards,

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

iD8DBQFCnLbULD1OTBfyMaQRArmUAKD8v6H424DG0POXEl7sNAfC3GKWzwCdF4U7
Zzt7J1nnG24jYChR8DKE7Jg=
=3dJw
-END PGP SIGNATURE-




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

Re: Windows 2000 - HOMEDRIVE,HOMEPATH vs. USERPROFILE

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

It sounds to me like we probably want to ignore USERPROFILE, then, and
just use the home dir settings.  How does the home directory get set to
something other than "undefined" in your examples?  Is there an API to
read it from the system or must all applications rely on malleable
environment variables for this information?

Cheers,

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

iD8DBQFCnK6RLD1OTBfyMaQRAnWqAKC/KHPySouyvYPIcS+eCfXAOnRXWgCfbjT0
whrOdCPGQpoGKGn1J85SME8=
=4gdk
-END PGP SIGNATURE-




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

Re: Feature Branch - windows-NT/pwd.h & windows-NT/unistd.h

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>Hi Derek,
>
>File "windows-NT/pwd.h" declares several functions that
>are declared in "unistd.h" according to the NetBSD man
>pages.
>
>Since I created "windows-NT/unistd.c" do you object in
>principle to migrations from "pwd.c" to "unistd.c"?
>
>If you agree, I'll enumerate the functions to migrate
>for review before actually doing so.


That's fine.  If the function has a counterpart in  on UNIX, I
likely won't have a problem with any such migration you propose.

Cheers,

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

iD8DBQFCnI+kLD1OTBfyMaQRAjtUAJ9pdcUy4+wWBcQ3itMPrT61EUyraQCg10oF
idmDhjfxUlMKzCTHeOW/l4Q=
=5rNC
-END PGP SIGNATURE-




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

Re: windows-NT/pwd.c - struct passwd - Home Directory

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>What I don't understand is why "." is a reasonable home directory according
>to the "getpwuid" implementation:
>
> static char *home_dir = "."; /* we feel (no|every)where at home */


I don't agree that it is.  I think it was a poor, arbitrary
implementation decision at some point in the past before Windows had the
concept of a home dir, which did not show up until NT, I believe.

>My thought was along the lines of:
>
> struct passwd *
> getpwuid (int uid)
> {
> pw.pw_name = getlogin ();
>- pw.pw_dir = home_dir;
>+ pw.pw_dir = get_homedir ();
> pw.pw_shell = login_shell;
> pw.pw_uid = 0;
>
> return &pw;
> }


If the getpwuid function in windows-NT/pwd.c worked, then the
get_homedir in src/filesubr.c might be moved into src/subr.c and shared
with the Windows implementation, or so I was thinking.

Regardless, I do think this basic idea is correct.  My reconsideration
is that perhaps all user-settable environment variable references,
should be in get_homedir, and getpwuid would need to return the "secure"
home directory, as it is defined to on UNIX, where it is often read from
the /etc/passwd file, for instance.  The closest thing might be the
value from the registry on Windows.  Windows must read this from
somewhere in order to set USERPROFILE, HOMEDRIVE, and HOMEPATH on boot. 
You need to read the Windows specifications and determine where these
values are coming from to implement this correctly.  Is there no
equivalent to this function on Windows, where the home dir value may
have come from a domain server or whatever?

The UNIX specification for getpwuid is here:
.

>Another thought can take use one step closer to using "src/filesubr.c"
instead
>of "windows-NT/filesubr.c" by defining "wnt_homedir" in "woe32.c":


This would be fine if we decide the proper place for all env var lookups
is in get_homedir.

>A grep of "get_homedir" usage (below) leads me to believe that whichever way
>we choose, functions "getpwuid" and "get_homedir" should present a
consistent
>view of where the home directory is on a given platform.


Once again, to summarize, if getpwuid is to be implemented,
functionality should be divided as follows:

getpwuid: to POSIX spec as much as possible, secure source if possible,
output not user settable
get_homedir: to CVS requirements, uses env vars, possibly set by user

You should be able to get POSIX specs for the other functions in
windows-NT/pwd.c from opengroup.org as well.  Try changing the
"getpwuid" in the URL above to other function names as needed.

:)

Regards,

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

iD8DBQFCnI8ILD1OTBfyMaQRArjFAKCEmq0ApQTuiZKSjEIAsR/q2D/DNQCfW6sq
hERtcrUGiZhhgTBwUBWNhKE=
=62BD
-END PGP SIGNATURE-




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

Re: Feature Branch Windows Build Broken - lib/glob.c & WINDOWS32

[Derek Price]

Conrad T. Pino wrote:

>I suggest using %USERPROFILE% only as the next fall back for cases where any
>of %HOMEDRIVE%%HOMEPATH% are missing.
>  
>

I suggest some more research may be in order here first.  Make sure that
setting %USERPROFILE% isn't the perscribed method for overriding
%HOMEDRIVE%%HOMEPATH% when you want to use a network login or somesuch. 
If this is the case then %USERPROFILE% would need to have priority...

>We've documented:
>
>   %HOME%
>  
>

It might be worth reconsidering %HOME% for a general, i.e. GNULIB or
GLIBC glob, implementation.  See below.

>If all of %HOME%, %HOMEDRIVE%%HOMEPATH%, %USERPROFILE% are missing then we
>are indeed down to very improbable cases.  I introduced %ALLUSERSPROFILE%
>only as another fallback that's a better alternative to hard coded values.
>  
>

Again, I'm not sure I agree here.  If you expected ~soandso/*.c to
search soandso's home directory for C sources, would you be happier if
your program told you, "No such user `soandso'", or happily returned
with a list of C sources that came from who-knows-where.  I think most
users would prefer the easier to spot error message in the problem
case.  I think a big part of the task here is simply going to be getting
the order-of-precedence for potential home directory values straight on
Windows.

>There is no environment variable to locate the "Default User" profile.
>I consider that only because the current "glob.c" hard coded value is a
>now obsolete reference to that profile.
>
>The registry is the only way to locate the "Default User" profile and
>the default root directory for creating local profiles.
>
>I'm not proposing we use these and enumerated them as alternatives to
>consider.  Earlier in this message I committed to preparing as "list of
>possibilities" and will includes these for discussion purposes.
>  
>

Can you justify that using a "Default User" directory is the correct
thing to do when the logged in or requested user's directory cannot be
found?  I suspect this may be another case where an error message is
better, but perhaps you have a use case on Windows I am unfamilar with?

>1. You proposed modifying "pwd.c" but when I searched for HOME, HOMEDRIVE
>& HOMEPATH references I found this in "windows-NT/filesubr.c":
>
>   char *
>   get_homedir ()
>   {
>   static char *pathbuf;
>   char *hd, *hp;
>
>   if (pathbuf != NULL)
>   return pathbuf;
>   else if ((hd = getenv ("HOME")))
>   return hd;
>   else if ((hd = getenv ("HOMEDRIVE")) && (hp = getenv ("HOMEPATH")))
>   {
>   pathbuf = xmalloc (strlen (hd) + strlen (hp) + 5);
>   strcpy (pathbuf, hd);
>   strcat (pathbuf, hp);
>
>   return pathbuf;
>   }
>   else
>   return NULL;
>   }
>
>which looks like the natural place to make the modifications.
>  
>

I disagree.  pwd.c is implementing several standard UNIX/POSIX APIs for
locating information about a logged in or other user.  As such, they
seem like the likliest candidates for acceptance into GNULIB.  Such a
module could be plugged directly into a UNIX app on Windows and
potentially allow it to compile and work there.

Also, since %HOME% is non-standard on Windows, perhaps it is best to
leave %HOME% in the CVS get_homedir function, and implement whatever
seems to make standard sense on Windows in the pwd.c stuff.  When we
submit it to GNULIB/GLIBC, we can mention that we made this choice in
case there is disagreement.

>2. In the "get_homedir" implementation shown above, disposal of the "xmalloc"
>provided "pathbuf" is moot since it's allocated once and process termination
>cleans up all messes.  Do you agree?
>  
>

Yes.

>3. The "windows-NT/pwd.c" implementation is broken because it does NOT call
>the "get_homedir" function.  Do you agree?
>  
>

No.  Again, pwd.c should implement the UNIX/POSIX APIs.  get_homedir can
wrap CVS functionality we think it is unlikely others will wish to
share, like consulting %HOME%.

>4. Should we back port these changes to stable branch?
>  
>
I don't think so.  You are welcome to explore the code paths and try to
determine where the pwd.c stubs may have been disabling functionality on
Windows such that fixing pwd.c should be considered a bug fix, but I am
inclined to call all of this new functionality.

It is probably worth noting that even in glob.c, these windows-NT/pwd.c
fixes wouldn't fix much in CVS.  The glob is being used to find files in
the repository and for the new HistorySearchPath config option.  Only in
the second case could processing `~' occur and I'm not sure it is
particularly useful.  Of course, that's not to say that GNULIB and GLIBC
might not value the contribution.

Cheers,

Derek



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

Re: Module "run.c" Interface Standardized - Feature To Stable Branch

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>I'll implement at least Option 2 by Wednesday. Do you have any preferred
>language or points you'd want covered in the "FIXME" note?


Nothing in particular.  Have at it.

Cheers,

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

iD8DBQFCm5FkLD1OTBfyMaQRAt79AKCuZL4zTBKRpmBRYNCEPlPjjpdYqQCeIpNK
A157dndVzb7yQq/1HFrO4dA=
=788F
-END PGP SIGNATURE-




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

Re: OpenBSD 3.6 sparc64 stable crash

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Alexander Taler wrote:

> Core was generated by `cvs'.
> Program terminated with signal 10, Bus error.
> #0 0x0014f148 in run_add_arg_p (iargc=0x7a8000,
> iarg_allocated=0x7a8004, iargv=0x7a7ff8, s=0x293838 "diff")
> at /home/cvstest/cvs-nightly-cvs1-11-x-branch/src/run.c:81
> 81 if (*iargc >= *iarg_allocated)


Why would the dereference of an int * or a size_t * or the comparison of
the resulting int and size_t cause a bus error?  Your stack  trace makes
it look like there are valid addresses in the inputs (iargc=0x7a8000,
iarg_allocated=0x7a8004) and this same code is compiling and passing
test on 7 or more other platforms, including NetBSD 1.6.1 and BSD/OS
though, as far as I know, none of the platforms are 64-bit.

Can you be a little more specific about the problem?  Perhaps loading
the core back into your debugger and trying to print some values (e.g.
iargc, *iargc, iarg_allocated, *iarg_allocated), might provide more clues.

Regards,

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

iD8DBQFCmzi/LD1OTBfyMaQRAsXyAJ9yaDIHvIBY5njvFV6f3dBFbDvxQQCgpunU
tJnHHSFgVjHZyTXRDIf/SEk=
=vLzc
-END PGP SIGNATURE-




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

Re: Module "run.c" Interface Standardized - Feature To Stable Branch

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>1. Who is able to run the OS/2 feature and stable builds?


I don't know.  Ocassionally, someone pokes their head in and sends us a
patch to get it up and running, but I think it's been at least a year
since that has happened.

>2. If no one steps forward to assist with building should
>we attempt a fix that can't be tested nevertheless?


The policy, as I thought was documented in HACKING or somesuch, is that
we do not bother.  I can't find the original text in a quick scan, however.

In general, if there is no way to tell if it works, what is the point? 
In reality, I ocassionally cut and paste a new function or API change
when I change it elsewhere, but more often lately, I don't bother.  If
someone decides to fix the platform up, it should be easy enough to
trace the missing functions to their counter-parts in src, lib. or wherever.

Cheers,

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

iD8DBQFCmlwmLD1OTBfyMaQRAqHtAKCtAxvzfZs0P2NUZvsPfXcUxRiyEwCgghx+
CDzN9M0fdJ8XuA1afbezSd8=
=+LUk
-END PGP SIGNATURE-




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

Re: CVS update [cvs1-11-x-branch]: /ccvs/windows-NT/

[Derek Price]

Conrad T. Pino wrote:

>Hi Derek,
>
>  
>
>>From: Derek Price
>>
>>There is an xnrealloc function in lib/xmalloc.c.  Isn't that being
>>compiled on Windows?
>>
>>
>
>No, stable branch Windows build omits it because it doesn't exist.
>My tests (see below) say "lib/xmalloc.c" is on feature branch only.
>  
>

Ah, okay.  Sorry.  I already forgot that I put this diff bugfix into
stable and didn't read the tag on your commit.  I had to make that same
change to the src/run.c on stable when I ported the change from feature.  :)

Thanks,

Derek



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

Re: CVS update [cvs1-11-x-branch]: /ccvs/windows-NT/

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

[EMAIL PROTECTED] wrote:

>Log:
> * run.c: Substitute "xrealloc" function for "xnrealloc" call from
> "run_add_arg_p" function.


There is an xnrealloc function in lib/xmalloc.c.  Isn't that being
compiled on Windows?

Cheers,

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

iD8DBQFCmSgyLD1OTBfyMaQRAgWmAJ4lGLn+3E3KJ1SUvE/uG3UwNOFqfQCgmcvV
5rwGa2ryMxFCJwlCKlkX1bM=
=7SI3
-END PGP SIGNATURE-




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

Re: Feature Branch Windows Build Broken - lib/glob.c & WINDOWS32

[Derek Price]

Conrad T. Pino wrote:

>I propose we do both, I'll edit and test, you coach:
>  
>

Sure.

>I'm OK with leaving WINDOWS32 undefined in our project.  I'd like to see
>an improvement in the WINDOWS32 code for the benefit of other projects if
>for no other reason than saying thank you to GLib and GNULib by sending
>back some useful code.
>
>I will improve "pwd.c" also so long as cutting and pasting is acceptable
>between "pwd.c" and "glob.c" is acceptable.
>  
>

Hrm?  Cutting and pasting in which direction.  You can try and get the
pwd.c module into GNULIB it might work.  I just ran across the GNULIB
execute module which seems to have some Windows specific code.  The
macros stuff is already in GNULIB, so if you could abstract some of the
Windows specific stuff out into those macros, I think it can only simplify.

>The bit of work concerns me only if we have an open issue that breaks the
>Windows build.  If the Windows build works I'll work on this issue.
>  
>

I believe the Windows build was working last night and probably is again
after my recent commit..

>I assume "inadvertantly fix...former" is the "pwd.c" improvement.  If so
>I share your motivation and agree.
>  
>

Yes.

>>Do you really expect this to be set if %USERPROFILE% is not?  It depends
>>on it...
>>
>>
>
>Windows NT, 2000, XP and 2003 will set the values.  I expect trouble only when
>the user has used command window to undefine or redefine which is unlikely.
>  
>

I'm not sure why earlier CVS developers missed the %USERPROFILE% value
then, unless it was missing in earlier versions of NT, but if you say
so...  Anyhow, if we leave %HOMEDRIVE%%HOMEPATH% as a fallback value, it
shouldn't be a big deal to add hte new handling.

>No "ALLUSERSPROFILE" does not depend on "USERPROFILE" and you are correct on
>it's use as shorthand to illustrate they typically have a common root.
>  
>

Why even process ALLUSERSPROFILE here?  As long as %USERPROFILE% and
%HOMEDRIVE%%HOMEPATH% are susually set, then if they are unset then
something is wrong, I would think.  It just doesn't sound like a
reasonable fallback to me.  The UNIX fallback, or even "secure" method
is to read the /etc/passwd file.  Maybe the closest fallback on Windows
is to read the registry?

>Power users to modify the registry can relocate their profile away from the
>default profile root.
>
>Networked users with file server resident profiles is another case.
>  
>

True.  Is %USERPROFILES% the best we can do here?

>Windows has no support for "~" and similar.  We can provide it as a reference
>to "$HOME", "$USERPROFILE" and/or "$HOMEDRIVE", "$HOMEPATH" combination.
>  
>

As part of glob, and to support consistent usage in CVS config files,
supporting "~" still seems reasonable.  Determining what that means is
up to us.  CVS supported the non-standard $HOME on Windows 95/98/ME
because there was nothing standard.  It is possible that continuing to
support that for glob is fine sine it won't normally be set anyhow, but
what we should aim at is whatever seems morally closest in Windows.


>>>+  if (home_dir == NULL || home_dir[0] == '\0')
>>>+  {
>>>+  /*"$SystemDrive/users" is Windows NT 4 specific
>>>+
>>>+NEW INSTALLS of Windows 2000 and later use
>>>+"$SystemDrive/Documents and Settings"
>>>+
>>>+UPGRADES use previous location
>>>+
>>>+The default user profile can't be found with an 
>>>environment
>>>+variable.  It's location is in the Windows 
>>>registry.
>>>+
>>>+The SystemDrive environment variable is an 
>>>alternative.
>>>+*/
>>>+  home_dir = "c:/users/default"; /* poor default */
>>>+  }
>>>  
>>>
>>CVS already uses %HOMEDRIVE%%HOMPATH% on windows (set automatically for
>>a long time on NT and carried through to XP), which sounds like another
>>good possibility.  I think it might predate %USERPROFILE%, so
>>%USERPROFILE% should probably be checked first.  It might be nice to
>>roll this all together into one place, like the pwd.c stuff, to avoid
>>having different portions of hte program do different things on Windows
>>with respect to $HOME.  Then the get_homedir() function from
>>windows-NT/filesubr.c should just check %HOME% then make the getpwname
>>(getlogin()) call.
>>
>>
>
>I can't say which came first but it's not particularly relevant to me.
>  
>

Newest is probably best, from the standpoint of supporting what
Microsoft declares most current first.  Supporting the legacy stuff in
chronologically reverse order should manage to support both old systems
and those who unset their %USERPROFILE% to allow their old setup, which
knew how to deal with %HOMEDRIVE%%HOMEPATH%, or whatever, to work.

Is %USERPROFILE% and %HOMEDRIVE%%HOMEPATH% really the same thing?  Now
that you have me talking about it, old memories that say the
%USERPROFIL

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

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>breaks the Windows build since Microsoft does NOT provide "sys/cdefs.h"
>implementation.


Yes.  Since we don't run configure on Windows, _SYS_CDEFS_H needed to be
defined to 1 in the windows-NT/config.h.in.in.  I've done so and
installed it.

Regards,

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

iD8DBQFCmPCxLD1OTBfyMaQRAqfrAKDcNY5MOH8PnOYdID9dZgqNxj8b6wCgy3E6
ELXlzCQfccQvTI7rfH3GpY0=
=EgB+
-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:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>>Fair enough, but why undo the change to glob.m4?  Shouldn't I just
>>change the target of the AC_DEFINE from MISSING_SYS_CDEFS_H to _SYS_CDEFS_H?
>>
>>
>
>Yes, you're right.
>
>Sorry, I'd forgotten the trick that I had suggested.  (This suggests
>that it deserves a nice fat comment.  :-)
>  
>

Done.  I added this comment to both glob_.h & glob.m4, with different
comment leaders, of course:

/* Note the reversal of the common HAVE_SYS_CDEFS_H idiom below.  In this
   way, #ifndef _SYS_CDEFS_H may be used to include  both when
   it has been checked for via the GNULIB configure test and found and when
   it has not been checked for, which we can presume means that the 
   GNULIB shares with GLIBC is being included as a system header and not as
   part of GNULIB, in which case  may be assumed.  */

I'm going to commit what I have as the new glob module soon if noone
says otherwise.  I and the CVS team made a few more minor changes to
make it work on Solaris, BSD, and Windows, but it is now compiling and
running on some 7 diverse platforms.

After that I will write up a ChangeLog entry for the glob-glibc2gnulib
diff and submit our changes back to the glibc team, unless someone here
who is used to working with them would like to take a go at the actual
submission part.  Perhaps it would be smoother if someone already known
to the glibc team introduced me and this patch?  Is the "definitive"
version of shared files usually in glibc, gnulib, or wherever someone
last remembered to merge when this happens?

Cheers,

Derek



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

Patch to protect spaces in diff arguments.

[Derek Price]

I just committed the attached patch to the stable branch.  This problem
has been listed in BUGS for some time, so I felt justified despite it
being a fairly large patch.  I went to some lengths to keep it clean and
I think that it mostly actually simplifes the code.

Cheers,

Derek
Index: BUGS
===
RCS file: /cvs/ccvs/BUGS,v
retrieving revision 1.55.6.18
retrieving revision 1.55.6.19
diff -u -p -r1.55.6.18 -r1.55.6.19
--- BUGS25 Aug 2004 03:14:01 -  1.55.6.18
+++ BUGS27 May 2005 16:25:41 -  1.55.6.19
@@ -88,13 +88,6 @@ an incorrect value, though this does not
 noticed under BSDI.
 
 
-* Spaces in arguments to `cvs diff' are currently split on spaces and tabs
-before being passed to diff.  This can often cause diff to abort since it can
-no longer interpret its options string and if it can, coincidentally,
-interpret its option string, then the problem may be output in unexpected
-formats.
-
-
 * Status
 
  /*---.
Index: ChangeLog
===
RCS file: /cvs/ccvs/ChangeLog,v
retrieving revision 1.692.2.209
retrieving revision 1.692.2.210
diff -u -p -r1.692.2.209 -r1.692.2.210
--- ChangeLog   4 May 2005 02:35:14 -   1.692.2.209
+++ ChangeLog   27 May 2005 16:25:41 -  1.692.2.210
@@ -1,3 +1,8 @@
+2005-05-27  Derek Price  <[EMAIL PROTECTED]>
+
+   * NEWS: Note diff space split fix.
+   * BUGS: Remove diff space split note.
+
 2005-05-03  Derek Price  <[EMAIL PROTECTED]>
 
* INSTALL: Add footnote about compiling a CVS checkout of CVS on a
Index: NEWS
===
RCS file: /cvs/ccvs/NEWS,v
retrieving revision 1.116.2.131
retrieving revision 1.116.2.132
diff -u -p -r1.116.2.131 -r1.116.2.132
--- NEWS2 May 2005 17:07:20 -   1.116.2.131
+++ NEWS27 May 2005 16:25:41 -  1.116.2.132
@@ -3,6 +3,8 @@ Changes since 1.11.20:
 
 BUG FIXES
 
+* `cvs diff' no longer splits its arguments on spaces.
+
 * Thanks to an old report and patch from Stewart Brodie <[EMAIL PROTECTED]>, a
   potential crash in response to a corrupt RCS file has been fixed.
 
Index: src/ChangeLog
===
RCS file: /cvs/ccvs/src/ChangeLog,v
retrieving revision 1.2336.2.371
retrieving revision 1.2336.2.373
diff -u -p -r1.2336.2.371 -r1.2336.2.373
--- src/ChangeLog   2 May 2005 19:39:10 -   1.2336.2.371
+++ src/ChangeLog   27 May 2005 16:28:00 -  1.2336.2.373
@@ -1,3 +1,25 @@
+2005-05-27  Derek Price  <[EMAIL PROTECTED]>
+
+   * client.c (send_arg): Make arg const.
+   (send_option_string): Rename to...
+   (send_options): ...this and accept argc/argv in place of string.
+   * client.h: Update protos to match the changes to client.c.
+   * cvs.h (RCS_exec_rcsdiff, diff_exec): Update protos.
+   (run_add_arg_p, run_arg_free_p): New protos.
+   * diff.c (opts, opts_allocated): Replace with...
+   (diff_argv, diff_argc, diff_arg_allocated): ...these.
+   (add_diff_args): New convenience function.
+   (diff): Use new constructs and APIs.
+   * patch.c (patch_fileproc, RCS_checkin, RCS_delete_revs), rcscmds.c
+   (call_diff_add_arg, call_diff_setup, RCS_merge, RCS_exec_rcsdiff,
+   diff_exec, RCS_output_diff_options), update.c (patch_file): Use new
+   APIs.
+   * run.c (run_add_arg_p, run_arg_free_p): New functions.
+   (run_argc_allocated): Make size_t.
+   (run_setup, run_add_arg): Use new functions.
+   * sanity.sh: Accomodate above changes.
+   (rcslib-diffrgx-3): Slip in test for space splitting.
+
 2005-05-02  Derek Price  <[EMAIL PROTECTED]>
 
Remove unnecessary level of indirection.
Index: src/client.c
===
RCS file: /cvs/ccvs/src/client.c,v
retrieving revision 1.318.4.27
retrieving revision 1.318.4.28
diff -u -p -r1.318.4.27 -r1.318.4.28
--- src/client.c17 Mar 2005 15:39:09 -  1.318.4.27
+++ src/client.c27 May 2005 16:25:10 -  1.318.4.28
@@ -4903,10 +4903,10 @@ start_rsh_server (root, to_server, from_
 /* Send an argument STRING.  */
 void
 send_arg (string)
-char *string;
+const char *string;
 {
 char buf[1];
-char *p = string;
+const char *p = string;
 
 send_to_server ("Argument ", 0);
 
@@ -5404,36 +5404,15 @@ send_dirleave_proc (callerdat, dir, err,
 }
 
 /*
- * Send each option in a string to the server, one by one.
- * This assumes that the options are separated by spaces, for example
- * STRING might be "--foo -C5 -y".
+ * Send each option in an array to the server, one by one.
+ * argv might be "--foo=bar",  "-C", "5", "-y".
  */
-
 void
-send_option_string (str

extensions.m4 patch (was Re: [bug-gnulib] New GNULIB glob module?)

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Paul Eggert wrote:

>--- extensions.m4.~1.6.~ 2005-02-23 05:49:36 -0800
>+++ extensions.m4 2005-05-24 12:35:48 -0700
>@@ -21,6 +21,10 @@ AC_DEFUN([gl_USE_SYSTEM_EXTENSIONS], [
> [/* Enable extensions on Solaris. */
> #ifndef __EXTENSIONS__
> # undef __EXTENSIONS__
>+#endif
>+#ifndef _POSIX_PTHREAD_SEMANTICS
>+# undef _POSIX_PTHREAD_SEMANTICS
> #endif])
> AC_DEFINE([__EXTENSIONS__])
>+ AC_DEFINE([_POSIX_PTHREAD_SEMANTICS])
> ])


I've installed this since it worked as advertised.

Cheers,

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

iD8DBQFClgh2LD1OTBfyMaQRAgWtAJ9OPSu9ONkQPFzzdXu8u+DSZadXaQCgsTWK
kIxtjhOf4KIbO/mVbj+jpEc=
=091c
-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:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>>I chose the _SYS_CDEFS_H route since it seemed simplest to me, though I
>>chose to name the macro `MISSING_SYS_CDEFS_H'.
>>
>>
>
>Sorry, that's not right, since it fails in the following scenario:
>
>  #define MISSING_SYS_CDEFS_H 27
>  #include 
>
>in an ordinary program that is not using gnulib, but is using glibc.
>MISSING_SYS_CDEFS_H is part of the user's namespace, and should not affect
>how  works.
>
>Let's just use _SYS_CDEFS_H.  We can then remove all mention of
>MISSING_SYS_CDEFS_H from glob_.h and undo the most recent change to
>glob.m4.  This should be OK: if we run into some non-glibc that has
>this problem it will probably use some other include file anyway.
>  
>

Fair enough, but why undo the change to glob.m4?  Shouldn't I just
change the target of the AC_DEFINE from MISSING_SYS_CDEFS_H to _SYS_CDEFS_H?

Regards,

Derek



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

Re: Feature Branch Windows Build Broken - lib/glob.c & WINDOWS32

[Derek Price]

Conrad T. Pino wrote:

>Yes, the Windows build completes.  Can you explain what "home_dir" value
>will be in the "WINDOWS32" undefined logic?
>  
>

It looks like it would call, basically, getpwnam (getlogin()).  There
are substitutes for both in windows-NT/pwd.c, though it doesn't look
like they are doing much useful.

>How about defining "WINDOWS32" and using the patch below?
>  
>

Maybe this would be preferable.  This would enable some of the [a-z]:/
swithcing other places in the file as well.  Of course, it's possible
much of that could be abstracted out using the FILE_SYSTEM_PREFIX_LEN &
ISSLASH macros.

My feeling is that it might be nice to move as much of your patch as
possible into pwd.c, abstract out as much of the drive letter prefix
stuff as possible into the aformentioned macros, skip defining
WINDOWS32, and see how things look, but it might be a bit of work.  If
you'd rather take the easy way out, I won't argue, but the former might
inadvertantly fix a few other hitherto unnoticed or ignored bugs on Windows.

>>Is it too much to hope for that Windows has  and implements
>>the readdir() family of functions and defining HAVE_DIRENT_H would be
>>enough here?
>>
>>
>
>Yes, that's entirely too much to hope for.  The Windows build fakes an
>"ndir.h" implementation in the "windows-NT" director and I committed a
>patch to "lib/glob_.h" that supports the "ndir.h" flavor.
>
>The #if logic was cut from "lib/glob.c" lines 53 through 71 with the
>extras discarded.  My patch is a quick hack.  I would appreciate it if
>you choose to make improvements.
>  
>

No, I think what you did is fine, at least until more than one #ifdef is
needed.  The version in glob.c is nice for making the rest of the
dirent/direct switching pretty transparent, but it's not needed yet.

>Index: lib/glob.c
>===
>RCS file: /cvs/ccvs/lib/glob.c,v
>retrieving revision 1.15
>diff -u -p -r1.15 glob.c
>--- lib/glob.c 25 May 2005 20:23:38 -  1.15
>+++ lib/glob.c 26 May 2005 10:57:36 -
>@@ -524,8 +525,45 @@ glob (const char *pattern, int flags,
>   home_dir = "SYS:";
> # else
> #  ifdef WINDOWS32
>+/* Windows doesn't set HOME, honor it if user sets it */
> if (home_dir == NULL || home_dir[0] == '\0')
>-home_dir = "c:/users/default"; /* poor default */
>+{
>+/* Windows sets USERPROFILE like UNIX sets HOME */
>+home_dir = getenv( "USERPROFILE" );
>+}
>+if (home_dir == NULL || home_dir[0] == '\0')
>+{
>+/* Windows sets APPDATA to "$USERPROFILE/Application Data" */
>+home_dir = getenv( "APPDATA" );
>+}
>  
>

Do you really expect this to be set if %USERPROFILE% is not?  It depends
on it...

>+if (home_dir == NULL || home_dir[0] == '\0')
>+{
>+/* Windows sets ALLUSERSPROFILE to "$USERPROFILE/../All 
>Users" */
>+home_dir = getenv( "ALLUSERSPROFILE" );
>+}
>  
>

Does this really depend on %USERPROFILE% too or is this just your
shorthand to refer to the user-profile directory?

>+if (home_dir == NULL || home_dir[0] == '\0')
>+{
>+/* Windows sets SystemRoot to installation value typically
>+   C:/WinNT but frequently C:/Windows */
>+/* This may be a bad idea but it's an alternative to the root 
>*/
>+home_dir = getenv( "SystemRoot" );
>+}
>  
>

I'm not sure how I feel about using SystemRoot as a fallback for ~ or
~username...  sounds unintuitive and possibly dangerous.

>+if (home_dir == NULL || home_dir[0] == '\0')
>+{
>+/*"$SystemDrive/users" is Windows NT 4 specific
>+
>+  NEW INSTALLS of Windows 2000 and later use
>+  "$SystemDrive/Documents and Settings"
>+
>+  UPGRADES use previous location
>+
>+  The default user profile can't be found with an 
>environment
>+  variable.  It's location is in the Windows 
>registry.
>+
>+  The SystemDrive environment variable is an 
>alternative.
>+  */
>+home_dir = "c:/users/default"; /* poor default */
>+}
>  
>

CVS already uses %HOMEDRIVE%%HOMPATH% on windows (set automatically for
a long time on NT and carried through to XP), which sounds like another
good possibility.  I think it might predate %USERPROFILE%, so
%USERPROFILE% should probably be checked first.  It might be nice to
roll this all together into one place, like the pwd.c stuff, to avoid
having different portions of hte program do different things on Windows
with respect to $HOME.  Then the get_homedir() function from
windows-NT/filesubr.c should just check %HOME% then make the getpwname
(getlogin()) call.

Regards,

Derek



_

Re: Feature Branch Windows Build - lib/dup-safer.c & dup

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>Hi All,
>
>The "dup" function call in "lib/dup-safer.c" has no
>prototype included. Windows Visual C 6.0 does NOT
>implement "dup" but does implement "_dup" as:
>
> int _dup( int handle );
>
>I've added "#define dup _dup" to "config.h" chain
>but Microsoft provides the prototype in ""
>which is NOT referenced in "lib/dup-safer.c" and
>perhaps should be.


Are you willing to take this up on GNULIB?

>I don't know the "m4" stuff and can't provide the
>complete solutions. Suggestions are welcome.


This won't matter since the configure stuff doesn't run on Windows
anyhow.  We just need to get the correct define, in this case probably
HAVE_IO_H, into config.h and only include it when it is present in
dup-safer.c.

>The warning is below and it could be ignored since
>the assumed and actual return types are the same.


Alternatively, you could just provide the prototype in
config.h.in.footer too.

Cheers,

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

iD8DBQFClOaWLD1OTBfyMaQRAgeeAKCW+h10H3ycXyKVp6x4c4zNASzlDACfZYuM
EKdxgEAkk+A4uJ95YOcxz8g=
=0WDE
-END PGP SIGNATURE-




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

Re: Feature Branch Windows Build Broken - lib/glob.c & WINDOWS32

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>Oops, I forgot the error message which follows.
>
>Compiling...
>glob.c
>h:\conrad\projects\cvs-1.12\lib\glob.c(535) : warning C4013: 'sysconf'
undefined; assuming extern returning int
>h:\conrad\projects\cvs-1.12\lib\glob.c(535) : error C2065:
'_SC_LOGIN_NAME_MAX' : undeclared identifier


I've checked in what I hope is a fix for this.

>h:\conrad\projects\cvs-1.12\lib\glob.c(1171) : warning C4133: ':' :
incompatible types - from 'struct direct *' to 'struct dirent *'
>h:\conrad\projects\cvs-1.12\lib\glob.c(1171) : warning C4133:
'initializing' : incompatible types - from 'struct dirent *' to
>'struct direct *'


Is it too much to hope for that Windows has  and implements
the readdir() family of functions and defining HAVE_DIRENT_H would be
enough here?

Cheers,

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

iD8DBQFClN+OLD1OTBfyMaQRAi6LAJ0WECXd7eJETb52st5XbAcwTPsCyACg61oc
X0Mj+ydzrG/osYJ1DCtmIWo=
=2GBK
-END PGP SIGNATURE-




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

Re: Feature Branch Windows Build Broken - lib/canonicalize.c - ELOOP & lstat

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>>>-#define CVS_STAT wnt_stat
>>>-int wnt_stat ();
>>>-
>>>-#define CVS_LSTAT wnt_lstat
>>>-int wnt_lstat ();
>>>+int wnt_stat (const char *file, struct wnt_stat *sb);
>>
>>This may prove unnecessary. See below.
>
>
>I didn't see anything pertinent below. What did I miss?


The new stat module is going to define gl_stat and gl_lstat and provide
protos for rpl_stat and rpl_lstat in a header.  The easiest thing to do
is probably going to just use the new header and rename the
windows-NT/filesubr.c functions to rpl_stat and rpl_lstat.

Also, the stat function in glob.c is not using the timestamps, which is
all the windows-NT/filesubr.c functions mess with, so it will probably
be okay to leave its call to stat (as opposed to gl_stat) alone.

Anyhow, dealing with this should probably wait until Bruno commits his
changes to the stat module and I can import the new version from GNULIB.

Regards,

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

iD8DBQFClOMyLD1OTBfyMaQRAvk2AJ92UnrvZa0t7EkMSGwUnRYDtc5V/QCg2+EB
Smt2Qff7F7CeziS2jhOanoM=
=+Ysi
-END PGP SIGNATURE-




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

Re: Feature Branch Windows Build Broken - lib/glob.c & WINDOWS32

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>Hi Derek,
>
>The latest "lib/glob.c" introduced new macros not addressed
>in the m4 stuff. Of interest are "WINDOWS32" and perhaps
>"__MSDOS__" as "#define" and "#undef" respectively.
>
>Should I be expecting an "m4" solution which means I edit


No.

>"windows-NT/config.h.in.in" or not which means I edit the
>"windows-NT/config.h.in.footer" instead?


This stuff was cut and pasted from the glibc glob.c file.  Isn't
__MSDOS__ defined automatically by some compilers?  Considering that I
cannot find WINDOWS32 in any of the glibc header files, I would expect
WINDOWS32 to be defined similarly.

Looking at the code that is switching on those symbols, it looks mostly
like directory stuff to allow C:/ style paths, so if it turns out not to
be defined by your compiler it should be pretty harmless to just define
one or the other for now there.  I'm not so sure about the CONVERT_D_INO
stuff and REAL_DIR_ENTRY stuff.

Regards,

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

iD8DBQFClNzILD1OTBfyMaQRAuSaAKDfMOkgRHtBMgGWgDqIgtUS/WA1bACg8OYB
yCCGQzMwcmEGpQQ6SeW6iv4=
=oD74
-END PGP SIGNATURE-




___
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

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>I loaded the gcc 2.95 compiler for the sole purpose of compiling CVS on
>this platform since previously CVS Home didn't offer a binary for this
>platform. I loaded the gcc 3.4 compiler for the sole purpose of fixing
>the CVS compile on this platform. I assume both versions can coexist
>with appropriate configuration. I turn the questions around:


Does what I just checked in work for you on Solaris?

>How can we use both versions to the CVS Project's advantage?
>Which version should be canonical for next binary release?


With luck, it doesn't matter now.

Regards,

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

iD8DBQFClNl3LD1OTBfyMaQRAuw0AKCNgFRpezLlLdiqkdB20fWAgTc+YwCeK08B
0w1T6Fqgvd+DX2ysEtKdNAQ=
=Zx0Q
-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]

Larry Jones wrote:

>Derek Price writes:
>  
>
>>Larry, can you tell us if defining
>>_POSIX_PTHREAD_SEMANTICS would work to get the POSIX version of
>>getpwnam_r on Solaris?
>>
>>
>
>It looks like it.
>  
>

I've committed Paul's patch to the CVS CVS tree, as well as removing the
associated glob.c changes, and I will install it in GNULIB if it passes
CVS nightly testing on Solaris.

Regards,

Derek



___
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]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>>+# ifdef HAVE___POSIX_GETPWNAM_R
>>+ /* Solaris. */
>>+# define getpwnam_r(name, bufp, buf, len, res) \
>>+ __posix_getpwnam_r (name, bufp, buf, len, res)
>>+# endif
>
>
>I don't see why this is needed. The Solaris include files already
>contain the magic necessary to convert getpwnam_r to
>__posix_getpwnam_r, right? Or is it because that depends on
>_POSIX_PTHREAD_SEMANTICS? If so, why not define that, as follows? I
>don't see why any gnulib-using application would want the nonstandard
>Solaris versions of the *_r functions.
>
>--- extensions.m4.~1.6.~ 2005-02-23 05:49:36 -0800
>+++ extensions.m4 2005-05-24 12:35:48 -0700
>@@ -21,6 +21,10 @@ AC_DEFUN([gl_USE_SYSTEM_EXTENSIONS], [
> [/* Enable extensions on Solaris. */
> #ifndef __EXTENSIONS__
> # undef __EXTENSIONS__
>+#endif
>+#ifndef _POSIX_PTHREAD_SEMANTICS
>+# undef _POSIX_PTHREAD_SEMANTICS
> #endif])
> AC_DEFINE([__EXTENSIONS__])
>+ AC_DEFINE([_POSIX_PTHREAD_SEMANTICS])
> ])


I really have no idea.  Larry, can you tell us if defining
_POSIX_PTHREAD_SEMANTICS would work to get the POSIX version of
getpwnam_r on Solaris?

Regards,

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

iD8DBQFClKxWLD1OTBfyMaQRAot/AKDvKB48aoCE0kys6bB3zxx0KT06sACgijeV
z/hTtSVzW3MRjWbCzAZy7pg=
=4CER
-END PGP SIGNATURE-




___
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

[Derek Price]

Conrad T. Pino wrote:

>Hi Derek,
>
>  
>
>>From: Conrad T. Pino
>>
>>
>>
>>>From: Derek Price
>>>
>>>Perhaps the problem is in your GCC installation or usage?
>>>  
>>>
>
>A gcc upgrade sure helps.  I installed gcc 3.4.2 binary from SunFreeWare.\
>  
>

I brought up changes to the GNULIB stat module up on bug-gnulib and
found out that this problem has been encountered before.  GNULIB is
going to continue supporting the older gccs without the fix you found in
3.4.2, so I have updated the glob.c file to use struct_stat64 in place
of struct stat, and #defined struct_stat64 to struct stat64 or struct
stat as appropriate.  Does this work for you with whichever compiler you
choose to stick with?

Regards,

Derek



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

Re: Feature Branch Windows Build Broken - lib/canonicalize.c - ELOOP & lstat

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>Hi Derek,
>
>Here's a rough draft of a patch that allows Windows build to complete.
>The draft includes edited files only and ignores generated files and
>Windows build projects files are also omitted.
>
>Briefly the plan is:
>
> #define stat wnt_stat
> #define lstat wnt_stat
>
> int wnt_stat (const char *file, struct wnt_stat *sb);
>
> Delete wnt_lstat definition


Are stat and lstat really equivalent on Windows?

>Index: lib/system.h
>===
>RCS file: /cvs/ccvs/lib/system.h,v
>retrieving revision 1.76
>diff -u -p -r1.76 system.h
>--- lib/system.h 24 May 2005 21:05:37 - 1.76
>+++ lib/system.h 25 May 2005 01:20:36 -
>@@ -371,7 +371,7 @@ extern int errno;
> otherwise return it unchanged. */
> #define convert_blocks(b, k) ((k) ? ((b) + 1) / 2 : (b))
>
>-#ifndef S_ISLNK
>+#if !defined(lstat) && !defined(S_ISLNK)
> # define lstat stat
> #endif


I suspect this was only relevant on Windows anyhow and the entire block
can be removed.

>Index: windows-NT/config.h.in.footer
>===
>RCS file: /cvs/ccvs/windows-NT/config.h.in.footer,v
>retrieving revision 1.4
>diff -u -p -r1.4 config.h.in.footer
>--- windows-NT/config.h.in.footer 1 Mar 2005 14:37:39 - 1.4
>+++ windows-NT/config.h.in.footer 25 May 2005 01:20:37 -
>@@ -11,11 +11,7 @@
> #define CVS_MKDIR wnt_mkdir
> int wnt_mkdir (const char *PATH, int MODE);
>
>-#define CVS_STAT wnt_stat
>-int wnt_stat ();
>-
>-#define CVS_LSTAT wnt_lstat
>-int wnt_lstat ();
>+int wnt_stat (const char *file, struct wnt_stat *sb);


This may prove unnecessary.  See below.

> #define CVS_RENAME wnt_rename
> int wnt_rename (const char *, const char *);
>@@ -95,3 +91,6 @@ char *sock_strerror (int errnum);
> /* getpagesize is missing on Windows, but 4096 seems to do the right
> thing. */
> #define getpagesize() 4096
>+
>+/* Windows has no ELOOP value in errno.h */
>+#define ELOOP 1


How about #define ELOOP EMLINK?  At least if this error ever comes up,
sterror will then produce a somewhat meaningful error message on Windows.

>Index: windows-NT/config.h.in.in
>===
>RCS file: /cvs/ccvs/windows-NT/config.h.in.in,v
>retrieving revision 1.30
>diff -u -p -r1.30 config.h.in.in
>--- windows-NT/config.h.in.in 2 May 2005 14:08:18 - 1.30
>+++ windows-NT/config.h.in.in 25 May 2005 01:20:37 -
>@@ -92,8 +92,9 @@
> message to be appended to the temp file when the editor is started. */
> #undef FORCE_USE_EDITOR
>
>-/* Define if gettimeofday clobbers localtime's static buffer. */
>-#undef GETTIMEOFDAY_CLOBBERS_LOCALTIME_BUFFER


Why are you removing these lines?

Cheers,

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

iD8DBQFClJehLD1OTBfyMaQRAghmAJ92Y1HxX1mz9Nr5N/vGtEQeu3XORQCfaFP1
5XRVx2RxZSdqN4YiqT1jrsw=
=I2jp
-END PGP SIGNATURE-




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

Re: Feature Branch Windows Build Broken - lib/canonicalize.c - ELOOP & lstat

[Derek Price]

Conrad T. Pino wrote:

>Hi Derek,
>
>  
>
>>From: Derek Price
>>
>>I've just checked in a patch which replaces all references to "CVS_STAT"
>>with "stat" and all references to "CVS_LSTAT" with "lstat".  I've made
>>changes to the GNULIB modules to support this and submitted the changes
>>back to GNULIB.  I also checked them into CVS to speed up our resolution
>>of this issue.
>>
>>
>
>I'm sorry to see CVS_STAT and CVS_LSTAT go.  They provided an abstraction
>that made CVS less platform bound.  The change makes us consistent with
>GNU Lib which wouldn't be a problem if they were open to Windows native
>API calls in GNU Lib code.
>  
>

Actually, because of errors similar to the ones you've been seeing on
Solaris, it sounds like GNULIB will be defining similar gl_stat and
gl_lstat macros.  I will either make the canonicalize module use those
and depend on the stat module or we can define both stat and gl_stat for
the windows port.

>>If you are feeling particularly motivated to ramp up the Windows support
>>in GNULIB, you could try to package the work wnt_stat and wnt_lstat do
>>on Windows into the GNULIB stat module and submit the whole back to
>>bug-gnulib@gnu.org, but it shouldn't be necessary and there has been
>>serious resistance there to adding anything to GNULIB modules like the
>>GetUTCFileModTime Windows system call that check_statbuf appears to be
>>making.
>>
>>
>
>I'm willing to take on only what's possible and your opinion counts:
>
>Does GNULIB include the Windows platform in it's charter?
>  
>

Sometimes.  How are your arm wrestling skills?  :)

>If yes, what's your take on the resistance to Windows native API calls?
>  
>

Windows native API call resistance was pretty strong last time I came up
against it, but GNULIB team members were willing to suggest compromises
that at least compiled on Windows.

>Since our Windows support is "client" mode only do loops matter?
>  
>

Yes, to the extent that the Windows client will support a local
repository.  It may be true that the loops are impossible on Windows
since links are not processed in the same way.

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

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>Defining macro "__PRAGMA_REDEFINE_EXTNAME" on the command line stops all
errors
>and warnings in "lib/glob.c" compile.


(For those new to this thread, this is because the Solaris headers
#define stat stat64 when the __PRAGMA_REDEFINE_EXTNAME macro is not
defined).

Why isn't gcc defining this value?  According to what I could find on
the web, the compiler should define "__PRAGMA_REDEFINE_EXTNAME" if it
supports the redefine_extname pragma, which this compiler apparently does.

.

Why haven't we seen this error before?  Is this a common problem on
Solaris 10?  Is this problem true of any gcc on any Solaris?  Is it
specific to your setup?  What happens if you use a native Soalris compiler?

A quick google search on "__PRAGMA_REDEFINE_EXTNAME Solaris 10 gcc"
brings up a few discussions of this.  The gist of the first few I read
is that some people ocassionally end up with broken GCCs which do not
properly define this macro, but most GCCs should.  The reason for the
difference was not apparent in the first several pages I scanned.

Digging a little deeper, this is even documented in the Linux info
manual for gcc:

> Solaris Pragmas
> ---
>
> For compatibility with the SunPRO compiler, the following pragma is
> supported.
>
> `redefine_extname OLDNAME NEWNAME'
> This pragma gives the C function OLDNAME the assembler label
> NEWNAME. The pragma must appear before the function declaration.
> This pragma is equivalent to the asm labels extension (*note Asm
> Labels::). The preprocessor defines `__PRAGMA_REDEFINE_EXTNAME'
> if the pragma is available.


Perhaps the problem is in your GCC installation or usage?

Regards,

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

iD8DBQFCk+etLD1OTBfyMaQRAs9VAJ9KrtbzdH5ikvAiviVanqIfsSQOZwCglN4c
RbnHOR4yXu7wBbG9E9s5P4U=
=78Ar
-END PGP SIGNATURE-




___
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

[Derek Price]

Conrad T. Pino wrote:

>Hi Derek,
>
>  
>
>>From: Derek Price
>>
>>
>>
>>>1. #include  within glob_.h file.
>>>  
>>>
>>This would be the way to go.  You may commit this if you wish.
>>
>>
>
>I committed an inclusion immediately before "struct stat" forward
>declaration to assure the Solaris "stat" macro is used but on 2nd
>reflection the forward declaration is moot.  I'm considering it's
>removal.  Any objection on your part?
>  
>

No objections here.  Please do so.  Also, please shorten this comment to
something like "get struct stat":

/* Solaris may "#define stat stat64" in some cases */

Thanks,

Derek



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

Re: Feature Branch Windows Build Broken - lib/canonicalize.c - ELOOP & lstat

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>Hi All,
>
>The new module "lib/canonicalize.c" fails to compile because it references
>the symbolic error "ELOOP" which Windows does not define.
>
>The new module "lib/canonicalize.c" calls the "lstat" function and there
>is no prototype and no implementation on Windows platform we support.
>
>We use "CVS_LSTAT" macro in the "src" directory which maps to "wnt_lstat"
>which we implement in "windows-NT/filesubr.c". We also have "lib/stat.c"
>module.


I've just checked in a patch which replaces all references to "CVS_STAT"
with "stat" and all references to "CVS_LSTAT" with "lstat".  I've made
changes to the GNULIB modules to support this and submitted the changes
back to GNULIB.  I also checked them into CVS to speed up our resolution
of this issue.

>Currently the Windows build doesn't use the "lib/stat.c" module.


I think it shouldn't.  the GNULIB stat and lstat modules replace stat
and lstat when some specific UNIX bugs are encountered, but rely on the
underlying stat & lstat to work.  The simplest thing to do at this point
would probably just be to #define stat wnt_stat and #define lstat
wnt_lstat in windows-NT/config.h.in.in.

If you are feeling particularly motivated to ramp up the Windows support
in GNULIB, you could try to package the work wnt_stat and wnt_lstat do
on Windows into the GNULIB stat module and submit the whole back to
bug-gnulib@gnu.org, but it shouldn't be necessary and there has been
serious resistance there to adding anything to GNULIB modules like the
GetUTCFileModTime Windows system call that check_statbuf appears to be
making.

>The questions raised appear to be:
>
>1. Where should an "ELOOP" definition be placed?
>
>2. How should the "ELOOP" value be selected?


These are good questions.  Is there any similar errno macro on Windows
to ELOOP?  ELOOP happens on UNIX when a program encounters symbolic
links like this:

ln -s dir2 dir1(dir1 --> dir2)
ln -s dir1 dir2(dir2 --> dir1)

and then a function is asked to evaluate a path containing one of these
elements, like `./dir1/sdir/file'.  Is something similar possible with
Windows links?  If so, there should be a "correct" substitute for ELOOP
on Windows.

>An incomplete patch set to the Windows build files is below to allow the
>Visual Studio IDE build to complete up to the EXE link. I plan to commit
>a complete fix once a consensus on the above issues emerges.


The ELOOP definition in your patch may not be the best, but your build
file changes should be fine.

Cheers,

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

iD8DBQFCk5i/LD1OTBfyMaQRAmpzAKCtKaFhT+SCkBBAyBiZqVdDMh8oqACgngpx
VyNsMm+TNZhbrpY6s6CKUs8=
=BD3e
-END PGP SIGNATURE-




___
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

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

It looks like your header is using some sort of #pragma to redefine the
function names, which I would hope would make them look less like macros
and actually remap the functions, allowing us to use the macros in
glob.c as they stand.

You are still encountering this problem with the latest glob.c?  We were
encountering problems using the *64 structs and functions on HP-UX and I
rewrote the file this morning to attempt to let compiler magic take care
of the 64 bit conversion, which is supposed to work transparently on all
systems.

Regards,

Derek


Conrad T. Pino wrote:

>Hi Derek,
>
>This platform's "sys/stat.h" implementation in this case
>defines macros for "stat", "fstat" and "lstat" to be the
>tokens "stat64", "fstat64" and "lstat64" respectively.
>My analysis of "sys/stat.h" relevent section is below.
>
>These macro definitions are in effect when "struct stat"
>definition is encountered which becomes "struct stat64".
>
>Line 187 of "lib/glob.c" redefines "stat64" as:
>
> # define stat64 stat
>
>which causes later "struct stat" references to be become
>"struct stat64" which becomes "struct stat" which is an
>empty forward declaration at best.
>
>It seems defining macros for "stat", "fstat", "lstat",
>"stat64", "fstat64" and "lstat64" on this platform is
>a risky proposition.
>
>Conrad
>
>/*
> * large file compilation environment setup
> */
>#if !defined(_LP64) && _FILE_OFFSET_BITS == 64
>// This is true #error Die
>#ifdef __PRAGMA_REDEFINE_EXTNAME
>// This is false #error Die
>#pragma redefine_extname fstat fstat64
>#pragma redefine_extname stat stat64
>
>#if (!defined(_POSIX_C_SOURCE) && !defined(_XOPEN_SOURCE)) || \
> defined(_XPG4_2) || defined(__EXTENSIONS__)
>// This is false #error Die
>#pragma redefine_extname lstat lstat64
>#endif
>#else /* __PRAGMA_REDEFINE_EXTNAME */
>// This is true #error Die
>#define fstat fstat64
>#define stat stat64
>#if (!defined(_POSIX_C_SOURCE) && !defined(_XOPEN_SOURCE)) || \
> defined(_XPG4_2) || defined(__EXTENSIONS__)
>// This is true #error Die
>#define lstat lstat64
>#endif
>#endif /* __PRAGMA_REDEFINE_EXTNAME */
>#endif /* !_LP64 && _FILE_OFFSET_BITS == 64 */
>

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

iD8DBQFCk5rLLD1OTBfyMaQRAhbVAJ4weUVLak0/PGsMQUPYxK+mVgy91gCeMJSV
Z/A53LJrStIsU8S9irOFLlY=
=MgP1
-END PGP SIGNATURE-




___
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

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>Hi Derek,
>
>We have a new "lib/glob.c" error today which I hope to
>understand before committing "#include "
>addition to "lib/glob_.h" you suggested.


Adding "#include " to glob_.h doesn't help with this new
error?  I wouldn't expect this error in glob.c anyhow, since it includes
 &  unconditionally.  According to POSIX.2,
struct stat should be defined in .  This same code, short of
a few recent changes, was compiling and passing tests on Solaris 8 the
other night.

Regards,

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

iD8DBQFCk3f2LD1OTBfyMaQRAtuHAJ98+uXWGRexlVslLWvfCE1+yCbMQQCgqvo6
JgiejX7f4tv94fJ5EEAz1Rk=
=9D5+
-END PGP SIGNATURE-




___
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

[Derek Price]

Conrad T. Pino wrote:

>1. #include  within glob_.h file.
>  
>

This would be the way to go.  You may commit this if you wish.

>2. Move #include  below #include  in glob.c file.
>  
>

This is incorrect.  The GNULIB convention is to include a module's
header immediately following the include of  in its associated
source file (i.e. glob.c first including config.h then immediately
including glob.h) to test the module's interface.

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

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>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?


Partly because the GNU glob semantics are more extensive than the POSIX
spec, partly because Mark Baushke reported that even glob functions that
are supposed to meet the POSIX spec often don't, the new glob module
only uses a system glob when it appears to be GNU glob.  I can't tell
for sure without reviewing your configure.log, but the specific reason
configure noticed that your glob.h doesn't work is probably the lack of
a  defining the _GNU_GLOB_INTERFACE_VERSION macro.

Cheers,

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

iD8DBQFCj7VRLD1OTBfyMaQRAjPjAKDIQrjsR+KcEDO+I1s5VvDLoq1uKgCg6zB2
TbTb97b7MY0vBc5ksWkTY6U=
=FvX6
-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: 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

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: 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

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

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>The Solaris function declaration from
>
> /usr/include/unistd.h:850: previous declaration of `getlogin_r'
>
>looks like:
>
> extern char *getlogin_r(char *, int);
>
>and "lib/glob.c" line 194 looks like:
>
> extern int getlogin_r (char *, size_t);


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.

Regards,

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

iD8DBQFCjSJNLD1OTBfyMaQRAu1zAKCQHUN7hUIRF3cg8ifF5SZhhMYCVgCgqVVM
qHJtNR8Jvse2ww0zgw/MR5s=
=H0VL
-END PGP SIGNATURE-




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

Re: history and val-tags locks.

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Mark D. Baushke wrote:

> For pserver, the administrator has full control over
> the command line.
>
> For server, if the administrator is using a
> restricted shell for users, they may or may not be
> able to restrict command-line arguments (it
> depends on how the restricted shell is
> implemented).


For server, OpenSSH, at the very least, allows the command line to be
restricted.  Are we really worried about the security of the command
line if the user is still using a vulnerable tool like RSH to access
their server?

> I have used (am using) a set-gid cvs and it does
> not disable the use of UNIX uids at all. Yes, it
> does inhibit gids as the entire repository uses
> the same gid ('cvs'), but the cvs_acls deals with
> permissions for commit and anyone with a login to
> the special-purpose CVS server has already been
> granted read permissions for checkout in any case.
>
> I will grant you that this is not necessarily a
> normal situation, but I make note of it as setuid
> is not the only configuration possiblity here.


Well, no, but for a secure setup, aren't we still recommending that the
admin rely on a transport such as SSH and filesystem permissions or ACLs
for access restriction in the repository?  The stated position of the
CVS developers has been that users should rely on systems that get more
thorough security audits than CVS does to provide the secure portions of
their setups.  This means using SSH as a transport, restricting the
command line, and relying on system permissions/ACLs to restrict access
to the various portions of a repository.

setuid and setgid CVS executables both disable the fine-grained access
restrictions that would otherwise be possible, limiting you, basically,
to one group per repository/server.

> As an alternative, I would not have a problem with
> allowing cvs to be built with a list of
> directories that could be searched for a valid
> config file. So, a --enable-config-prefix=/etc/cvs
> might mean that /etc/cvs/$CVSROOT/config would looked
> at before $CVSROOT/config ... doing that means that
> the administrator would still have complete control
> over the configurations and would not need to rely
> on the user to make the right choice.


This compromise does sound reasonable.  Would you then consider a
default value of `--enable-config-prefix=/etc' as reasonable here?  i.e.
the ability to override the config would default to `on', with access to
config files restricted to /etc and subdirs.

> The downside is that src/sanity.sh tests for this
> case would be more painful.


Well, we could test that the restricted paths were disabled, at least.

> As far as your src/sanity.sh tests, I believe you
> should use the -c CONFIG-FILE to determine if cvs
> has been configured with this option or not before
> you fail the tests (granted STABLE does not have
> that available to sanity.sh)


If it defaults to on, with config-prefix used as you suggest, would you
still consider this last important?

Regards,

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

iD8DBQFCi525LD1OTBfyMaQRAjERAKDuDciHLatseYmpN/PlQABI4Kcy7gCghAs8
KpNygtYAx/ZTx2+JWGjNaQI=
=G/xc
-END PGP SIGNATURE-




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

Re: Feature Branch Windows Build Broken - New GNULIB glob module

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>I updated with the recent commit of the glob module and we have a
>new warning on line 1184 of "lib/glob.c" file (see below)


Let's revisit this after you provide definitions for the following
macros in windows-NT/config.h.in.in:

../config.h.in line 101 has #undef GLOB_PREFIX not found in ./config.h.in.in
../config.h.in line 234 has #undef HAVE_DECL_STRDUP not found in
./config.h.in.in
../config.h.in line 297 has #undef HAVE_GETLOGIN_R not found in
./config.h.in.in
../config.h.in line 309 has #undef HAVE_GETPWNAM_R not found in
./config.h.in.in
../config.h.in line 533 has #undef HAVE_SIGINTERRUPT not found in
./config.h.in.in
../config.h.in line 548 has #undef HAVE_STAT64 not found in ./config.h.in.in
../config.h.in line 577 has #undef HAVE_STRDUP not found in ./config.h.in.in
../config.h.in line 607 has #undef HAVE_STRUCT_DIRENT64 not found in
./config.h.in.in
../config.h.in line 611 has #undef HAVE_STRUCT_DIRENT_D_TYPE not found
in ./config.h.in.in
../config.h.in line 632 has #undef HAVE_SYS_CDEFS_H not found in
./config.h.in.in


GLOB_PREFIX should be #defined to `rpl_'.  The rest of the macros are
looking for particular function declarations (prototypes), functions,
headers, or type definitions and need to be defined or undefined as such
are found or not on Windows with MSVC (or in the substitute functions in
windows-NT/*.[ch]).  HAVE_STRUCT_DIRENT_D_TYPE, in particular, wants to
know if struct dirent has a member named `d_type'.

Glancing at the lines that #define dirent direct in lib/glob.c, they
depend on the absence of .  Assuming that Windows has
 then, since it did not complain about a missing definition
for struct dirent, but only about a bad typecast, a #define
HAVE_DIRENT_H 1 may also be appropriate in config.h.in.in.

Cheers,

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

iD8DBQFCi2aELD1OTBfyMaQRAqQAAJ9bS1iHmJGQtBfs9dPxaKbxSX6aqwCeIDmB
Y4z8mNkcVDh+Yq2saXEQVHY=
=xVwO
-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]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Paul Eggert wrote:

>Answering my own question: yes it will, in general, because perhaps
>the gnulib version fixes a bug that's in the glibc version, and the
>user wants the gnulib version. So we should worry about this.
>And (as far as I can see) the simplest fix is to do this:
>
>#if defined _LIBC || HAVE_SYS_CDEFS_H
># include 
>#endif
>
>with the appropriate change to glob.m4 to define HAVE_SYS_CDEFS_H.


Done.  Patch delayed.

Regards,

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

iD8DBQFCi2ApLD1OTBfyMaQRAtapAJ47AVMRrJV5FEhKkybHBWaYRrcrXgCgspg9
PuWjxQRih2G3aP9omCCowAU=
=s6g2
-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:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>>>This seems a bit brittle.  Why not simply try to compile this program?
>>>
>>> #include 
>>> char a[_GNU_GLOB_INTERFACE_VERSION == 1 ? 1 : -1];
>>>  
>>>
>>Because I like to avoid runtime tests if I can avoid it, since they
>>cannot be used when cross-compiling.
>>
>>
>
>The code that I proposed does not have any runtime tests, since the
>array is allocated statically.  Tests like this are often used during
>cross-compiling.
>  
>

Ah, I didn't realize the optimizer could be relied on for code like
this.  Fixed.

Patch delayed until completion of today's email processing.

Regards,

Derek



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

Re: Feature Branch Windows Build Broken - New GNULIB glob module

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>I saw you're involved with Paul Eggert on a "New GNULIB module" topic
>and assumed we had done another GNULib import which has often broken
>the Windows build.


Well, basically.  I created the glob module myself and added it to CVS
and Paul has been reviewing it and I've been correcting it in ways to
make it compatible with GNULIB & GLIBC.

>I have my nose buried in another project. My patch is a blind guess
>since I didn't look at the UNIX build to see what is going on there.
>Your comments lead me to believe I made a lucky guess using "glob_.h"
>and compiling "glob.c" in the "libcvs" build.


I can't comment on how lucky you were, but you did arrive at the correct
solution.  :)

>I'm willing to commit the Windows build and "windows-NT/pwd.?" files.
>What's your preference on who sends "glob_.h" patch to GNULib team?


I'll do it.  It should slide into my ongoing discussion with Paul about
this new module easily enough.

>+#ifndef __restrict
>+# define __restrict
>+#endif


This is not correct.  It should be

#ifndef __restrict
# define __restrict restrict
#endif

If the "restrict" keyword is not supported by MSVC, then in
windows-NT/config.h.in.in, "restrict" should be defined to some
variation of "__restrict", "__restrict__", "_Restrict", or empty.

The rest of your patch looks fine.

Regards,

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

iD8DBQFCijCdLD1OTBfyMaQRAiT0AJ9Ddc7GePXgTuDhFl4WwVBRLebLfQCfViZR
Z5bC5MkdHep6RyAa8dR9dxE=
=c50w
-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]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Paul Eggert wrote:

>First, already we have something bogus: that __BEGIN_DECLS. It must
>be protected by "#ifdef _LIBC", since random C environments don't have
>it. Similarly for __END_DECLS.


Done.

>The simplest fix would be to do something like this:
>
>#if defined _LIBC || HAVE_SYS_CDEFS_H
># include 
>#endif
>
>with the appropriate change to glob.m4.


Actually, this change doesn't appear to be necessary once I protect the
__BEGIN_DECLS and __END_DECLS stuff.

I've removed the #include of .

>But let's step back a second. Why are we worried about building
>gnulib glob.c under glibc? It will never happen, right? So perhaps
>we needn't worry about this problem at all.


Won't it?  I thought the idea was that when you and I settled on
something that looked good I would attempt to send it back to the libc
folks so that the files wouldn't need to be maintained separately.  So
far, our work contains at the least some cleanup, a bug fix, and an
efficiency improvement that should be useful in libc.

>With that in mind, how about if we simply remove those #undef's? If
>the problem recurs we can put them back in again.


Done.

Patch still delayed until I'm done with my email.

Regards,

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

iD8DBQFCijnMLD1OTBfyMaQRAklUAKDo25YXS7Hk5kqlYe1AzXICeBBLfwCfahXg
oD3soIvk5Shqw50Mfm4dgRY=
=/2W5
-END PGP SIGNATURE-




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

Re: history and val-tags locks.

[Derek Price]

Derek Price wrote:

>Derek Price wrote:
>
>  
>
>>I see your point.  What about `cvs server'?  I can see both setups being
>>useful...  an admin who allowed users access to the CVS repository would
>>probably prefer not to allow the config file to be specified whereas an
>>admin who restriced the command that SSH users could run to a particular
>>shell script that provided the -c option wouldn't mind...  perhaps it
>>should be a compile time option, with the default to disallow it.
>>
>>
>
>
>On further consideration, if we are going to consider a configurable
>config path with other CVS modes a security risk, then using it with
>pserver has to be considered a security risk too.  There is nothing
>stopping a creative user with shell access to a machine from using
>pserver mode to access their repository.
>
>I might argue that any administrator worried that much about security
>should be disabling shell access to the machine anyhow, which would deal
>with any insecurity resulting from a configurable config path, but I
>don't feel so strongly about it that I wouldn't happily install it as a
>compile-time option that defaults to off.
>  
>


I've implemented this as an option to server & pserver.  Installing as a
global option would have create problems in multiroot mode anyhow.

Preliminary patch against 1.11.x attached.  The final version will go
into feature - I'm not advocating putting this in stable, but this is
what I have now and I thought I would request a review.  This patch also
finally disables the sourcing of the ~/.cvsrc file for the server
commands as an added protection against a user setting the path to the
config file.

2005-05-17  Derek Price  <[EMAIL PROTECTED]>

* configure.in: Add --enable-config-override.
* main.c (main): Don't source .cvsrc in server mode.  Remove
obsolete comment.
* parseinfo.c (ConfigPath): New global.
(parse_config): Open ConfigPath when provided.
* server.c (server): Parse -c option.
* sanity.sh (server_usage): New static global.
(sever): Add tests of ConfigPath and .cvsrc.
  

I've been thinking about this more, and I'm starting to feel that as an
option to server/pserver/etc, this really isn't so insecure.  In
general, an admin will be able to and probably does restrict the
arguments to the server & pserver commands, and a user with shell access
to the server could run a hacked CVS against a repo or even alter a repo
directly anyhow, so the argument about security is mostly moot.

The only exception would be where the admin only used a setuid CVS
executable to restrict repo access to a specific CVS executable.  I'm
not sure how common this is however, as it also disables the ability to
use UNIX uids & gids for finer control over read & write access.

Regards,

Derek



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

Re: history and val-tags locks.

[Derek Price]

Patch actually attached this time.

Cheers,

Derek


Derek Price wrote:

>I've implemented this as an option to server & pserver.  Installing as a
>global option would have create problems in multiroot mode anyhow.
>
>Preliminary patch against 1.11.x attached.  The final version will go
>into feature - I'm not advocating putting this in stable, but this is
>what I have now and I thought I would request a review.  This patch also
>finally disables the sourcing of the ~/.cvsrc file for the server
>commands as an added protection against a user setting the path to the
>config file.
>
>2005-05-17  Derek Price  <[EMAIL PROTECTED]>
>
>* configure.in: Add --enable-config-override.
>* main.c (main): Don't source .cvsrc in server mode.  Remove
>obsolete comment.
>* parseinfo.c (ConfigPath): New global.
>(parse_config): Open ConfigPath when provided.
>* server.c (server): Parse -c option.
>* sanity.sh (server_usage): New static global.
>(sever): Add tests of ConfigPath and .cvsrc.
>  
>
>I've been thinking about this more, and I'm starting to feel that as an
>option to server/pserver/etc, this really isn't so insecure.  In
>general, an admin will be able to and probably does restrict the
>arguments to the server & pserver commands, and a user with shell access
>to the server could run a hacked CVS against a repo or even alter a repo
>directly anyhow, so the argument about security is mostly moot.
>
>The only exception would be where the admin only used a setuid CVS
>executable to restrict repo access to a specific CVS executable.  I'm
>not sure how common this is however, as it also disables the ability to
>use UNIX uids & gids for finer control over read & write access.
>
>Regards,
>
>Derek
>  
>
Index: configure.in
===
RCS file: /cvs/ccvs/configure.in,v
retrieving revision 1.176.2.60
diff -u -p -r1.176.2.60 configure.in
--- configure.in18 Apr 2005 17:46:13 -  1.176.2.60
+++ configure.in17 May 2005 16:06:53 -
@@ -965,9 +965,32 @@ dnl end --enable-rootcommit
 dnl
 
 
+dnl
+dnl begin --enable-config-override
+dnl
+
+AC_ARG_ENABLE(
+  [config-override],
+  AC_HELP_STRING(
+[--enable-config-override],
+[Cause the CVS server commands to allow the config file to be specified
+ on the command line.  (enabled by default)]), ,
+  [enable_config_override=yes])
+
+if test x"$enable_config_override" = xyes; then
+  AC_DEFINE(ALLOW_CONFIG_OVERRIDE, 1,
+[Define this to allow the path to CVS's config file to be set on the
+ command line.])
+fi
+
+dnl
+dnl end --enable-config-override
+dnl
+
+
 
 dnl
-dnl end --enable-*
+dnl end --enables
 dnl
 
 
Index: src/main.c
===
RCS file: /cvs/ccvs/src/main.c,v
retrieving revision 1.172.4.14
diff -u -p -r1.172.4.14 main.c
--- src/main.c  9 Mar 2005 19:47:15 -   1.172.4.14
+++ src/main.c  17 May 2005 16:06:53 -
@@ -478,6 +478,17 @@ main (argc, argv)
use_cvsrc = 0;
 }
 
+#ifdef SERVER_SUPPORT
+/* Don't try and read a .cvsrc file if we are a server.  */
+if (optind < argc
+   && (!strcmp (argv[optind], "pserver")
+# ifdef HAVE_KERBEROS
+   || !strcmp (argv[optind], "kserver")
+# endif /* HAVE_KERBEROS */
+   || !strcmp (argv[optind], "server")))
+   use_cvsrc = 0;
+#endif /* SERVER_SUPPORT */
+
 /*
  * Scan cvsrc file for global options.
  */
@@ -693,10 +704,7 @@ distribution kit for a complete list of 
if (strcmp (cvs_cmd_name, "pserver") == 0)
{
/* The reason that --allow-root is not a command option
-  is mainly the comment in server() about how argc,argv
-  might be from .cvsrc.  I'm not sure about that, and
-  I'm not sure it is only true of command options, but
-  it seems easier to make it a global option.  */
+  is mainly that it seems easier to make it a global option.  */
 
/* Gets username and password from client, authenticates, then
   switches to run as that user and sends an ACK back to the
Index: src/parseinfo.c
===
RCS file: /cvs/ccvs/src/parseinfo.c,v
retrieving revision 1.37.4.8
diff -u -p -r1.37.4.8 parseinfo.c
--- src/parseinfo.c 16 Mar 2005 22:00:44 -  1.37.4.8
+++ src/parseinfo.c 17 May 2005 16:06:53 -
@@ -17,6 +17,9 @@
 #include "history.h"
 
 extern char *logHistory;
+#ifdef ALLOW_CONFIG_OVERRIDE
+char *ConfigPath;
+#endif
 
 /*
  * Parse the INFOFILE file for the specified REPOSITORY.  Invoke CALLPROC for
@@ -252,22 +255,24 @@ p

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

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>>Are you sure? You asked me to restore similar parens around bit-ands
>>back at several other locations despite other work that changed the
>>lines, in an earlier email. Not that I disagree now. I actually prefer
>>the version without the unnecessary parens around the bit-and. I just
>>think we should be consistent.
>
>
>It's a minor point, but expressions like (a & b && c) are assumed to
>be confusing, as they depend on obscure precedence rules, whereas
>expressions like (a & b ? c : d) are not confusing in the same way:
>there's only one way to parse them, even if you have forgotten the
>precedence.


Okay, parens removed.  Delaying new patch until I've processed the rest
of your emails.

Cheers,

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

iD8DBQFCii0TLD1OTBfyMaQRAgE0AKCMyzvr5P31kyXB2aDTDdUSOTp6HQCg2+dm
CjcxyJKKkqFEyexvpoMajtE=
=h/Cn
-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:

>Now for commentary on glob.h.
>
>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>>--- ../glibc-2.3.5/posix/glob.h   2004-09-16 20:55:15.0 -0400
>>+++ lib/glob_.h   2005-05-13 12:21:39.0 -0400
>>@@ -19,29 +19,48 @@
>> #ifndef  _GLOB_H
>> #define  _GLOB_H 1
>> 
>>-#include 
>>+#ifdef _LIBC
>>+# include 
>>+#else
>>+# include 
>>+# include 
>>+# undef __size_t
>>+# define __size_t size_t
>>+#endif
>>
>>
>
>Why do we need to include  here?  All we need is size_t,
>right?  And stddef.h gives us that.
>  
>

If I don't, I get the following error:

In file included from glob.c:23:
glob.h:107: error: syntax error before "struct"
In file included from /usr/include/errno.h:36,
 from glob.c:25:
/usr/include/bits/errno.h:38: error: syntax error before "extern"

Hrm.  Tracing this a little farther, it is only , or even
the , that glob_.h was originally including, and not
, which is needed to avoid the above error.  
appears to define __USE_GNU (which the glob_.h file uses) when
_GNU_SOURCE is defined, but a #define __USE_GNU prior to the include of
 isn't sufficient to do the trick.  Have you seen anything
like this before?  I can't really tell the difference between
 and  with a simple test since they each appear
to include the other.  I'm personally inclined to leave the #include
 since it is the standard header and it seems to fix
things, but perhaps you can give me more information?

>>+/* Some system libraries erroneously define these.  */
>>+#undef   GLOB_ERR
>>...
>>
>>
>
>Which system libraries are these?  Perhaps a comment?
>  
>

I don't know.  I copied and pasted this comment and the #undef block
from the glob.c file.  It was previously placed just prior to the
#include of .  This looked the better place for it since I
assumed that we wouldn't want applications using the GNULIB glob module
to issue lots of redefinition warnings.

>>+# define getopt __GLOB_ID (getopt)
>>
>>
>
>Surely this line is spurious and can be removed.
>  
>

Done.

>>-#if !defined __USE_FILE_OFFSET64 || __GNUC__ < 2
>>+#if !defined _LIBC || !defined __USE_FILE_OFFSET64 || __GNUC__ < 2
>>
>>
>
>Can't we remove at least the "|| __GNUC__ < 2" here?  glibc assumes
>recent versions of GCC.
>  
>

I'll take your word for it.  I've also followed through to remove the
following block:

#if __USE_FILE_OFFSET64 && __GNUC__ < 2
# define glob glob64
# define globfree globfree64
#endif

And replaced this block:

# ifndef __size_t
#  if defined __GNUC__ && __GNUC__ >= 2
typedef __SIZE_TYPE__ __size_t;
#   ifdef __USE_XOPEN
typedef __SIZE_TYPE__ size_t;
#   endif
#  else
#   include 
#   ifndef __size_t
#define __size_t size_t
#   endif
#  endif
# else

with this block:

# ifndef __size_t
typedef __SIZE_TYPE__ __size_t;
#  ifdef __USE_XOPEN
typedef __SIZE_TYPE__ size_t;
#  endif
# else

>> extern void __REDIRECT_NTH (globfree, (glob_t *__pglob), globfree64);
>> #endif
>>-
>> #ifdef __USE_LARGEFILE64
>> extern int glob64 (__const char *__restrict __pattern, int __flags,
>> int (*__errfunc) (__const char *, int),
>>
>>
>
>Let's omit this change; it's a different matter.
>  
>

Done.

Still withholding my patch.  I have one more email to process.

Regards,

Derek



___
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:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>> /* Enable GNU extensions in glob.h.  */
>>-#ifndef _GNU_SOURCE
>>+#if defined _LIBC && !defined _GNU_SOURCE
>> # define _GNU_SOURCE 1
>> #endif
>>
>>
>
>I just checked the glibc source file include/libc-symbols.h, and it
>defines both _LIBC and _GNU_SOURCE.  So this stuff isn't needed at
>all; please remove these lines from glob.c instead.
>  
>

Done.

>>+   HAVE_STRUCT_DIRENT_D_TYPE is defined by GNULIB's glob.m4 when the same
>>+   member is found.  */
>>
>>
>
>A bit wordy; please rephrase to "HAVE_STRUCT_DIRENT_D_TYPE plays the
>same rule in gnulib."
>  
>

Rephrased.

>>-  qsort ((__ptr_t) &pglob->gl_pathv[oldcount],
>>+  qsort ((void *) &pglob->gl_pathv[oldcount],
>>
>>
>
>You can remove the cast entirely, right?
>  
>

So you agreed in an earlier message.  I've now removed all typecasts to
and from void * which are declared unnecessary by C89.

>>-   free ((__ptr_t) pglob->gl_pathv[pglob->gl_offs + i]);
>>-  free ((__ptr_t) pglob->gl_pathv);
>>+   free ((void *) pglob->gl_pathv[pglob->gl_offs + i]);
>>+  free ((void *) pglob->gl_pathv);
>>
>>
>
>Similarly, these casts aren't needed.
>
>-  free ((__ptr_t) array[--i]);
>+  free ((void *) array[--i]);
>
>Nor these.
>
>  
>
>>- free ((__ptr_t) array[--i]);
>>+ free ((void *) array[--i]);
>>
>>
>
>Nor this one.
>
>  
>
>>+   p = mempcpy ((void *) new->name, name, len);
>>
>>
>
>Nor this one.
>
>  
>
>>- free ((__ptr_t) names->name);
>>+ free ((void *) names->name);
>>
>>
>
>Nor this.
>  
>

Removed, removed, removed, removed, and removed, per the above.

>>-  return (((flags & GLOB_ALTDIRFUNC)
>>-? (*pglob->gl_stat) (fullname, &st)
>>-: __stat64 (fullname, &st64)) == 0);
>>+  return ((flags & GLOB_ALTDIRFUNC)
>>+   ? (*pglob->gl_stat) (fullname, &st) == 0 && S_ISDIR (st.st_mode)
>>+   : __stat64 (fullname, &st64) == 0 && S_ISDIR (st64.st_mode));
>>
>>
>
>As long as you're reparenthesizing you might as well get rid of
>the unnecessary ones around (flags & GLOB_ALTDIRFUNC).
>  
>

Are you sure?  You asked me to restore similar parens around bit-ands
back at several other locations despite other work that changed the
lines, in an earlier email.  Not that I disagree now.  I actually prefer
the version without the unnecessary parens around the  bit-and.  I just
think we should be consistent.

>>+   new->name = malloc (len + 1
>>+   + ((flags & GLOB_MARK) && isdir));
>>
>>
>
>This line is 80 columns; should probably shrink it to 78, e.g., via.
>
>  new->name =
>malloc (...);
>  
>

Done.

>glob.c is looking pretty good now.  I gotta run now, but I'll look at
>glob.h later today I hope.
>  
>

Thanks.  I'm not attaching a patch - I'll include a complete one
shortly, after I process your next two emails.

Cheers,

Derek



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

Re: Feature Branch Windows Build Broken - New GNULIB glob module

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Conrad T. Pino wrote:

>The following patch lets me get this far in the Windows IDE build:
>Configuration: libcvs - Win32 Debug
>Compiling...
>glob.c
>H:\Conrad\Projects\cvs-1.12\lib\glob.c(253) : warning C4028: formal
parameter 1 different from declaration
>H:\Conrad\Projects\cvs-1.12\lib\glob.c(253) : warning C4028: formal
parameter 4 different from declaration
>H:\Conrad\Projects\cvs-1.12\lib\glob.c(668) : warning C4090: 'function'
: different 'const' qualifiers
>H:\Conrad\Projects\cvs-1.12\lib\glob.c(668) : warning C4024: 'getpwnam'
: different types for formal and actual parameter 1
>H:\Conrad\Projects\cvs-1.12\lib\glob.c(739) : warning C4013:
'glob_pattern_p' undefined; assuming extern returning int
>H:\Conrad\Projects\cvs-1.12\lib\glob.c(334) : fatal error C1600:
unsupported data type
>Error executing cl.exe.
>
>cvs.exe - 1 error(s), 5 warning(s)
>===
>The following patches do the following:
>
>1. Add WIN32 conditional #defines to "lib/glob_.h" to hide compile
>errors.
>
>2. Modify "lib/libcvs.dsp" to make "lib/glob.h" file
>from "lib/glob_.h" file.
>
>3. Modify Modify "lib/libcvs.dsp" to build "lib/glob.c" which is
>where the compile is failing after my hacks to "lib/glob_.h" file.
>===
>Index: lib/glob_.h
>===
>RCS file: /cvs/ccvs/lib/glob_.h,v
>retrieving revision 1.5
>diff -u -p -r1.5 glob_.h
>--- lib/glob_.h 15 May 2005 16:44:04 - 1.5
>+++ lib/glob_.h 17 May 2005 04:18:51 -
>@@ -19,6 +19,12 @@
> #ifndef _GLOB_H
> #define _GLOB_H 1
>
>+#ifdef WIN32
>+#define __BEGIN_DECLS
>+#define __END_DECLS
>+#define __const const
>+#endif


I'm going to suggest something a little more general to the GNULIB
maintainers:

#ifndef __BEGIN_DECLS
# define __BEGIN_DECLS
#endif
#ifndef __END_DECLS
# define __END_DECLS
#endif
#ifndef __const
# define __const const
#endif

I won't comment on the Windows build stuff - it sounds like you are
mirroring what the UNIX build is doing, but otherwise I prefer to stay
out of it.

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

iD8DBQFCigN0LD1OTBfyMaQRAnTvAJsHGOa5tqkD9crYZZ1/lFoYrsIS4QCgkChQ
eVG7KDjY6HnZUNK+LuIeUP0=
=FheJ
-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]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Thanks for the latest round.  I'm going to be out of the office today,
but I should get to it by tomorrow.

Regards,

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

iD8DBQFCiICvLD1OTBfyMaQRAsMbAJ96UgJcf/G+zBfOlXBJM+nBJzff/ACglQte
CYnnsSKWV74qlZJbIcVNNWk=
=0b5Z
-END PGP SIGNATURE-




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

Re: NEWS file needs update?

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Jim Hyslop wrote:

> According to this message,
http://lists.gnu.org/archive/html/info-cvs/2004-10/msg00252.html, the
"Protocol error: uncounted data discarded" error was scheduled to be
fixed in 1.11.18, but there is no mention of that error in the NEWS file.
>
> Is this simply an oversight in the NEWS file?


I believe so.  Usually, obscure, minor, and non-user visible changes get
lumped into the "Misc bug and documentation fixes."

Cheers,

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

iD8DBQFCg8M9LD1OTBfyMaQRAg2uAJsFTiICQ4BFtWK17DP7KSA2JQjTtwCglLvo
Hf3ATi4gxI6TG1dSdrxvdyQ=
=mNFx
-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]

Most of this looks good, but one quick question as I get to this:  I
assume from the following that I can expect _LIBC to be defined iff the
file is being compiled as part of glibc?  I wasn't sure...

Cheers,

Derek


Paul Eggert wrote:

>>-#ifdef _LIBC
>>-# include 
>>-# undef strdup
>>-# define strdup(str) __strdup (str)
>>-# define sysconf(id) __sysconf (id)
>>-# define closedir(dir) __closedir (dir)
>>-# define opendir(name) __opendir (name)
>>-# define readdir(str) __readdir64 (str)
>>-# define getpwnam_r(name, bufp, buf, len, res) \
>>-   __getpwnam_r (name, bufp, buf, len, res)
>>-# ifndef __stat64
>>-#  define __stat64(fname, buf) __xstat64 (_STAT_VER, fname, buf)
>>-# endif
>>-# define HAVE_STAT64 1
>>-#endif
>>
>>
>
>Please leave this sort of code in, so that the source code can be
>identical in libc.
>
>  
>
>>+#include "mempcpy.h"
>>+#include "stat-macros.h"
>>+#include "strdup.h"
>>
>>
>
>These includes need to be protected by #ifndef _LIBC.
>  
>



___
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]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Karl Berry wrote:

> Subject: [bug-gnulib] New GNULIB glob module?
>
>Would it be possible to simply use the libc code as-is? I guess I mean,
>with whatever changes are needed sent back to libc.
>
>So much stuff in gnulib is 95% the same as libc. It doesn't seem good.


Well, like I said, a bunch of the cruft I cut out could be removed
because it was much simpler to depend on other GNULIB modules.  The
alternative would have been to add a bunch of tests to detect the things
that were being switched on, and I expect would have been much more
complicated since those problems have already been solved in the GNULIB
modules the file now references and I would have had to test the new
tests...

To have libc use the same code, it would be best if they imported the
GNULIB modules my new glob depends on and then reimported my glob
files.  We might have to find a slightly better compromise on the header
file since I'm not sure that all the assumptions I made there that are
valid with GNULIB would be valid at run time for other programs
expecting only glibc.

(Glancing back at the header file now, it looks like I did a few things
that should be undone/fixed anyhow.  I reviewed my changes to glob.c
more thoroughly, but I disabled a few things in glob.h like the 64 bit
file offsets, intending to revisit the issue later.  I don't know all
the issues involved there...  what's "__REDIRECT_NTH" and why does it
break in my compiler?  I was hoping someone else might be able to deal
with that more quickly than I, but most of the rest of my changes should
be valid and useful as/in the port to GNULIB...)

Regards,

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

iD8DBQFCgoiULD1OTBfyMaQRApY+AJ9VsOAvoa3endmyHCxT1ai/+Nch4ACfWAye
yXShaWGtcwczl045kRTY7G4=
=Wf8s
-END PGP SIGNATURE-




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

Re: [bug-gnulib] Re: getopt and Solaris 10

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Karl Berry wrote:

> GNU getopt tries to do too much when it reorders the commandline and
> therefor needs the "+" as a workaround.
>
>I don't suppose it matters at this point, but I fail to see the
>connection here. You can tell GNU getopt to REQUIRE_ORDER instead of
>PERMUTE without using +, either directly or setting POSIXLY_CORRECT in
>various ways.


I'd rather not control a function I'm calling by messing with
environment variables, especially those that may have other side effects.

> See the __ordering enum in getopt_int.h, for example.
>(Seems like you probably already know this, so maybe it's unsuitable
>somehow, if so, sorry for the noise.)


It doesn't look to me like there is an easy way to set this from a
caller of getopt().

>Also, I don't agree that it is "doing too much" to have PERMUTE be the
>default for GNU. It is very useful for most programs. cvs has
>exceptional command-line parsing needs.


True.

Cheers,

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

iD8DBQFCgV4aLD1OTBfyMaQRAtYPAJ9q5KLtCt4T7JwYHNWSLqgPMKe2sgCg4D9I
OIkpN56K4vKZXgSvo0tFmJM=
=iNeT
-END PGP SIGNATURE-




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

Re: [bug-gnulib] Re: getopt and Solaris 10

[Derek Price]

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>  
>
>>Revised patch attached.
>>
>>
>
>Thanks; I installed the following slightly-different patch.
>  
>


Works for me.  Thanks Paul.

Derek



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

Re: lockinfo

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Mark D. Baushke wrote:

> I honestly don't know if it is a good idea to
> introduce this patch to feature in this way at
> this time.
>
> What do the other maintainers think of this kind
> of extension?


I don't have a strong opinion either way.  Some users might find it
useful to have read ACLs made available this way.  Me, I prefer to
offload any ACL decisions on the file system since it is almost
certainly more secure.

As long as I don't have to do the work, though, and without thinking
deeply about your discussion about whether soft or hard failures are
better, I have no objections to this patch.

Cheers,

Derek

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

iD8DBQFCgMRDLD1OTBfyMaQRAs1XAJ9Pu3LSUAoKAPaF67L8oiKlIfBuugCfXay/
t2xW87sMgKYXgcbk4bW4PdI=
=D8vS
-END PGP SIGNATURE-




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

Re: [bug-gnulib] Re: getopt and Solaris 10

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Matthias Kurz wrote:

>I'm running out of time. I have to quit. Thanks very much for your effords.
>When there is something to test, i'll still try to help.


No problem.  Thanks for your help, Matthias.

Regards,

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

iD8DBQFCgL7qLD1OTBfyMaQRAgNTAJ4yFV0txiOeuKI9/459BArLy7iVSACfYZhj
oP6Fu8IWbqICpJRFhSP376o=
=LDtS
-END PGP SIGNATURE-




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

Re: [bug-gnulib] Re: getopt and Solaris 10

[Derek Price]

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>
>  
>
>>+ myargv[[0]] = "conftest";
>>+ myargv[[1]] = "-+";
>>
>>
>
>This doesn't null-terminate myargv.
>  
>

Okay, looking at that in C89 now, but just out of curiosity, if argv
needs to be NULL terminated, what's the point of argc?

Revised patch attached.  I restored a comment I probably shouldn't have
cut as well.

2005-05-10  Derek Price  <[EMAIL PROTECTED]>

* m4/getopt.m4: Check for Solaris 10 bug, not decl, when possible.


Regards,

Derek
Index: m4/getopt.m4
===
RCS file: /cvsroot/gnulib/gnulib/m4/getopt.m4,v
retrieving revision 1.9
diff -u -p -r1.9 getopt.m4
--- m4/getopt.m46 May 2005 01:04:20 -   1.9
+++ m4/getopt.m410 May 2005 13:56:05 -
@@ -39,8 +39,26 @@ AC_DEFUN([gl_GETOPT],
 dnl Solaris 10 getopt doesn't handle `+' as a leading character in an
 dnl option string (as of 2005-05-05).
 if test -z "$GETOPT_H"; then
-  AC_CHECK_DECL([getopt_clip], [GETOPT_H=getopt.h], [],
-   [#include ])
+  AC_CACHE_CHECK([for working GNU getopt function], gl_cv_func_gnu_getopt,
+  [AC_RUN_IFELSE(
+[AC_LANG_PROGRAM([#include ],[
+ char *myargv[[3]];
+ myargv[[0]] = "conftest";
+ myargv[[1]] = "-+";
+ myargv[[2]] = NULL;
+ return '?' != getopt (2, myargv, "+a");
+])],
+gl_cv_func_gnu_getopt=yes,
+gl_cv_func_gnu_getopt=no,
+[dnl cross compiling - pessimistically guess based on decls
+ dnl Solaris 10 getopt doesn't handle `+' as a leading character in an
+ dnl option string (as of 2005-05-05).
+ AC_CHECK_DECL([getopt_clip],
+ [gl_cv_func_gnu_getopt=no], [gl_cv_func_gnu_getopt=yes],
+[#include ])])])
+  if test "$gl_cv_func_gnu_getopt" = "no"; then
+   GETOPT_H=getopt.h
+  fi
 fi
 
 if test -n "$GETOPT_H"; then
___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] Re: getopt and Solaris 10

[Derek Price]

Paul Eggert wrote:

>Derek Price <[EMAIL PROTECTED]> writes:
>
>
>  
>
>>+ myargv[[0]] = "conftest";
>>+ myargv[[1]] = "-+";
>>
>>
>
>This doesn't null-terminate myargv.
>
>But I still don't get why the change is needed.  It sounds like you're
>assuming Solaris 11 getopt might get fixed?  
>

Yes, I suppose so, or a later patch release of Solaris 10.  Or some
other system that decides to ship a getopt.h with a getopt_long(),
getopt_clip(), and a GNU compatible getopt(), however unlikely that may
sound now.

>But even in that case,
>the current code will work, right, since it will use GNU getopt?  So
>this is just an optimization for the hypothetical case if Solaris 11
>getopt gets fixed?  
>

Basically.  I was feeling guilty about not detecting the bug, but
detecting a feature of the system and assuming the bug.  This goes
against the whole Autoconf test design philosophy, I thought.  It's
almost as bad as using `uname -a |sed' to detect the version number and
using that to decide against the system getopt(), really.

The new test detects the actual bug.

>In that case perhaps we should wait for Solaris 11
>and test it before installing this patch, as it's more likely to cause
>a bug than to fix one.
>  
>

What sort of bug are you worried about?

Regards,

Derek



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

Re: [bug-gnulib] Re: getopt and Solaris 10

[Derek Price]

Derek Price wrote:

>>>2005-05-05  Paul Eggert  <[EMAIL PROTECTED]>
>>>
>>> * lib/getopt.m4 (gl_GETOPT): Check for Solaris 10 getopt, and
>>> avoid needless checks.   
>>>  
>>>
>>Yes, this also works for me.
>>
>>
>
>Okay, one more revision, to actually check if the -+ registers as an
>option or not.  This fits the autoconf paradigm of testing for the bug,
>even if it is using AC_TRY_RUN.  This way, if Sun fixes the problem in a
>later Solaris release, we shouldn't need to change anything.
>
>When cross-compiling, I fall back on looking for the odd Solaris decl to
>decide whether to use the GNULIB version of getopt or not.  If we wanted
>to be really pessimistic, we could always use the GNULIB getopt when
>cross-compiling, but I thought this could wait at least until we get a
>report of similar behavior from some system other than Solaris 10.
>
>Matthias, would you mind testing this version out?  If you could send me
>your config.log when you are done building, that would be helpful as well.
>
>2005-05-09  Derek Price  <[EMAIL PROTECTED]>
>
>* m4/getopt.m4: Check for Solaris 10 bug, not decl, when possible.
>  
>

I've attached almost the same patch as before, except that I remembered
to put `$' in front of all my variables this time.  Matthias confirmed
that ithis new patch does the right thing on Solaris 10.

This patch is against Paul's last commit.

Cheers,

Derek
Index: m4/getopt.m4
===
RCS file: /cvs/ccvs/m4/getopt.m4,v
retrieving revision 1.7
diff -u -p -r1.7 getopt.m4
--- m4/getopt.m46 May 2005 15:50:05 -   1.7
+++ m4/getopt.m49 May 2005 20:20:00 -
@@ -36,11 +36,26 @@ AC_DEFUN([gl_GETOPT],
   AC_CHECK_DECL([optreset], [GETOPT_H=getopt.h], [], [#include ])
 fi
 
-dnl Solaris 10 getopt doesn't handle `+' as a leading character in an
-dnl option string (as of 2005-05-05).
 if test -z "$GETOPT_H"; then
-  AC_CHECK_DECL([getopt_clip], [GETOPT_H=getopt.h], [],
-   [#include ])
+  AC_CACHE_CHECK([for working GNU getopt function], gl_cv_func_gnu_getopt,
+  [AC_RUN_IFELSE(
+[AC_LANG_PROGRAM([#include ],[
+ char *myargv[[2]];
+ myargv[[0]] = "conftest";
+ myargv[[1]] = "-+";
+ return '?' != getopt (2, myargv, "+a");
+])],
+gl_cv_func_gnu_getopt=yes,
+gl_cv_func_gnu_getopt=no,
+[dnl cross compiling - pessimistically guess based on decls
+ dnl Solaris 10 getopt doesn't handle `+' as a leading character in an
+ dnl option string (as of 2005-05-05).
+ AC_CHECK_DECL([getopt_clip],
+ [gl_cv_func_gnu_getopt=no], [gl_cv_func_gnu_getopt=yes],
+[#include ])])])
+  if test "$gl_cv_func_gnu_getopt" = "no"; then
+   GETOPT_H=getopt.h
+  fi
 fi
 
 if test -n "$GETOPT_H"; then
___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs

Re: [bug-gnulib] Re: getopt and Solaris 10

[Derek Price]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

...I know you already agreed...  ...I just thought this was interesting...

In fact, Solaris 10 is the one actually sticking to the letter of the
POSIX spec here, not that I think that assuming argv[0] should not be
processed is going to hurt the GNU version:

The variable /optind/ is the index of the next element of the
/argv/[] vector to be processed. It is initialised to 1 by the system,

...

If, when /getopt()/ is called:

|
 /argv/[optind]is a null pointer
*/argv/[optind]is not the character -
 /argv/[optind]points to the string "-"
|

 /getopt/() returns -1 without changing /optind./


Cheers,

Derek


Derek Price wrote:

>Regardless, since using an optind = 0 is not specified as supported by
>POSIX, whereas optind = 1 is, and since using optind = 1 in place of
>optind = 0 in CVS would avoid this problem on all platforms and with all
>versions of getopt (supporting optind = 0 provides no additional
>functionality that I know of), I don't think this is properly considered
>a Solaris bug, but a CVS bug.
>
>At least, it would be a CVS bug if we didn't have to use the GNU getopt
>on Solaris due to the "+" problem anyhow.
>
>Cheers,
>
>Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCf55NLD1OTBfyMaQRApD/AKDG91RXkmD1GjNyqLJmzFjQq9vCBwCfbjTw
HHeRY0WAPlT67fsFFn0PNuE=
=oFUI
-END PGP SIGNATURE-




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

Re: [bug-gnulib] Re: getopt and Solaris 10

[Derek Price]

Regardless, since using an optind = 0 is not specified as supported by
POSIX, whereas optind = 1 is, and since using optind = 1 in place of
optind = 0 in CVS would avoid this problem on all platforms and with all
versions of getopt (supporting optind = 0 provides no additional
functionality that I know of), I don't think this is properly considered
a Solaris bug, but a CVS bug.

At least, it would be a CVS bug if we didn't have to use the GNU getopt
on Solaris due to the "+" problem anyhow.

Cheers,

Derek


Derek Price wrote:

> Matthias Kurz wrote:
>
> >Hi.
>
> >One more clarification. Maybe one or the other missed my last comments
> >on <https://ccvs.cvshome.org/issues/show_bug.cgi?id=248>. It was me,
> >who introduced the "'+' myth".
>
>
>
> Well, it is not a myth.  I studied the test program and output you sent
> me, and Solaris does, indeed, treat the initial "+" as an option rather
> than a request for getopt() semantics.
>
> >I "analysed" the problem wrong in the
> >beginning. It is not exactly the "+" that makes the problem, but the
> >fact, that the Solaris getopt refuses to accept a zero value on optind.
> >When "optind==0", the Solaris getopt returns -1 immediately, leaving
> >optind==0. W
>
>
>
> I did read this correctly in your report.  If anyone else sees a need
> for an actual test for correct optind=0 behavior, then they are welcome
> to write one, but I decided the point was moot at the moment since
> detecting the "+" bug, just as valid as a means of selecting the GNU
> getopt() over the system one, was sufficient to avoid encountering any
> other bugs in the Solaris getopt().
>
> Cheers,
>
> Derek


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





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