Re: Feature request/ideas
| A rational way. As a second step I would suggest to change the | extensions (.prev, .next, .xxx) to be allowed in head position too, | but I'm note sure where the sandbox tag gets processed. If | translate_tags() could take that into account, its not a big deal | ... Where is this state cached? It's cached in the CVS/Entries files in each subdir. CVS looks it up in the Version_TS in vers_ts.c and decides to set it in the Vers_TS structure it returns only if the TAG passed to the function (via -r or something, somewhere) is not set. I think it could be caught there - if the TAG is .XXX (and not .trunk), then set the vers_ts-tag to STICKY.XXX. Ok, thats the 'client' side. But where is it cached on the 'server' side? | So probably the expression used should connote this. After some | consideration, I would vote for '.origin' here. I disagree with | being meaningless. I often export a project state into a local | repository, work on it, and when I'm done, move the files back to | the remote repository's sandbox. During local development I often | want to compare to the initial version of a file, and using a | single tag for this is just easy. Granted there are other ways to | achieve this, but they're not as easy to handle. That's fine for 1.1, but how does this help you for a branch? You might want to diff against the root, but it doesn't make much sense to care about the first revision on the branch. Good point. What about resolving '.origin' to the very first revision of the mainline? Regards Frank ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
lib/xtime.h is included twice
Dear cvs developers, for cvs 1.11.19 and earlier, lib/xtime.h is included twice, which breaks compiling on IRIX 5.3. The following patch prevents this: --- lib/xtime.h.orig2005-03-01 15:53:11.0 +0100 +++ lib/xtime.h 2005-03-01 15:54:20.0 +0100 @@ -12,6 +12,9 @@ * functions */ +#ifndef _XTIME_H_ +#define _XTIME_H_ + #ifdef vms # include time.h #else /* vms */ @@ -55,3 +58,5 @@ # endif /* !defined(HAVE_FTIME) !defined(HAVE_TIMEZONE) */ #endif /* !vms */ + +#endif /* !_XTIME_H_ */ -- Georg Schwarzhttp://home.pages.de/~schwarz/ [EMAIL PROTECTED] +49 178 8545053 ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
Derek Price wrote:
First, I found man pages stating that valloc() was obsoleted in favor of
posix_memalign(). Does anyone have a feel for how portable
posix_memalign() is
It appears to be implemented only by glibc.
Second, if posix_memalign() is not portable, is there any interest in a
GNULIB module either to replace it or valloc()? I've attached the
valloc.c that CVS is currently using.
The spec says that the returned memory must be freeable through free().
I don't see how you can write a portable replacement for posix_memalign()
without digging in undocumented details of the malloc() implementation.
void *
valloc (bytes)
size_t bytes;
{
long pagesize;
void *ret;
pagesize = getpagesize ();
ret = malloc (bytes + pagesize - 1);
if (ret)
ret = (long) (ret + pagesize - 1) ~ (pagesize - 1);
return ret;
}
This is the best you can do in a portable way, but
1. the return value cannot be passed to free(),
2. it wastes 1/2 page of memory on average.
Therefore I'd suggest a new interface:
void* pagealign_alloc(size_t);
void pagealign_free(void*);
and do the implementation as follows:
- If mmap() is available, use mmap and some bookkeeping for pagealign_alloc,
and munmap() for pagealign_free,
- Otherwise, if posix_memalign() is available, use it and free(),
- Otherwise, use something similar to the valloc() above.
Is this all worth it? For what purpose do you need the memory to be
page-aligned?
Bruno
___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
Derek Price wrote: Why mmap? mmap allocates the number of pages you want, without the 1/2 page lossage on average that malloc() has. What sort of object would you suggest I map the allocation to? My Linux man page says that the MAP_ANONYMOUS flag (which requires no object to map) has been supported since the 2.4 kernel, but I can find no reference to MAP_ANONYMOUS in the POSIX spec. Different systems want it differently. I use an autoconf test to determine which way to use mmap(): 1) int flags = MAP_ANON | MAP_PRIVATE; int fd = -1; 2) int flags = MAP_ANONYMOUS | MAP_PRIVATE; int fd = -1; 3) #ifndef MAP_FILE #define MAP_FILE 0 #endif int flags = MAP_FILE | MAP_PRIVATE; int fd = open(/dev/zero,O_RDONLY,0666); if (fd0) exit(1); These three together cover all systems that have mmap(). (Most of this is hardly necessary for CVS since it is already caching unused buffer datas rather than freeing them, and reusing them as they are needed. This is actually a bad idea, because 1) It steals memory from the system. If a program doesn't use memory but still holds it, it forces other programs to do more page faults. 2) When memory is tight, it forces the system to write garbage data to swap space. 3) It slows down 'cvs' itself. Because when the page gets swapped out to disk, and the the program notices that it needs it, the OS must re-fetch the page from the swap. This usually takes ca. 10 ms. Whereas when a program munmap()s the pages that it doesn't need any more, 1) Other programs can use the free mempry. 3) When the program needs more memory, the kernel can often provide a blank page immediately, without having to wait for a swapped out page to come in. Bruno ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
Derek Price wrote:
Why do you prefer mmap to posix_memalign?
Yes, if posix_memalign exists, it can probably be used instead of mmap.
But since the only known platform having it is glibc, and on glibc it
wastes even more memory than your simple valloc(), I would prefer raw mmap().
This simple program (on x86)
#include stdlib.h
int main () {
void* p;
posix_memalign(p, 4096, 2*4096);
posix_memalign(p, 4096, 10*4096);
posix_memalign(p, 4096, 100*4096);
return 0;
}
produces this strace:
brk(0) = 0x8049580
brk(0x804a580) = 0x804a580
brk(0) = 0x804a580
brk(0x804b000) = 0x804b000
brk(0) = 0x804b000
brk(0x804d000) = 0x804d000
brk(0) = 0x804d000
brk(0x8058000) = 0x8058000
mmap2(NULL, 417792, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x40156000
I.e. when asked for 2 pages, it allocates 2 pages.
when asked for 10 pages, it allocates 11 pages.
when asked for 100 pages, it allocates 102 pages.
it strikes me that my
bookkeeping for mmap could slow things down if many blocks were
allocated (I'm storing the ptr-size map in a simple linked list, but
even something more efficient would eventually slow).
There's a GPLed implementation of red-black trees in the Linux kernel.
Red-black trees, like AVL trees, have an O(log N) worst case runtime for
lookup, insertion, removal. I don't consider this slow.
Bruno
___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs
RE: lib/xtime.h is included twice
Georg Schwarz wrote: I cannot judge this. I was just following the example given in http://cvsweb.NetBSD.org/bsdweb.cgi/src/share/misc/style?rev=H EADcontent-type=text/x-cvsweb-markup Oh, dear. Time to send NetBSD a note suggesting they change the example, if they want to remain ISO/ANSI compliant. Can anyone confirm if [EMAIL PROTECTED] is the correct contact for this? If you think different naming conventions are more appropriate, I don't mind. If you're referring to the leading underscore, then that's not just a naming convention, it's a violation of the ANSI/ISO C standard (paragraph 7.1.3, Reserved Identifiers). If you're referring to the lower-case d, then I have no strong attachment to it. -- Jim Hyslop Senior Software Designer Leitch Technology International Inc. ( http://www.leitch.com ) Columnist, C/C++ Users Journal ( http://www.cuj.com/experts ) ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
RE: lib/xtime.h is included twice
[EMAIL PROTECTED] wrote: Dear cvs developers, for cvs 1.11.19 and earlier, lib/xtime.h is included twice, which breaks compiling on IRIX 5.3. The following patch prevents this: --- lib/xtime.h.orig2005-03-01 15:53:11.0 +0100 +++ lib/xtime.h 2005-03-01 15:54:20.0 +0100 @@ -12,6 +12,9 @@ * functions */ +#ifndef _XTIME_H_ +#define _XTIME_H_ ISO C Standard reserves names beginning with underscore and an uppercase character, or beginning with two underscores. I recommend changing this to something like: #ifndef XTIME_HEADER_INCLUDEd #define XTIME_HEADER_INCLUDEd (note: the lower-case d at the end is deliberate - it's my technique to reduce the chances of a name collision.) -- Jim Hyslop Senior Software Designer Leitch Technology International Inc. ( http://www.leitch.com ) Columnist, C/C++ Users Journal ( http://www.cuj.com/experts ) ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
Derek Price wrote:
Okay, I've implemented this as you suggested, Bruno. Installed in CVS,
it passes tests in all four modes (MMAP, MMAP/NO-MAP_ANON,
POSIX_MEMALIGN, OTHER). I've attached the patch, but I still have a few
questions.
Thanks. I've installed this into gnulib, with the following minor
modifications:
- Indentation according to GNU standards.
- pagealign_alloc.h:
- Add comments.
- Include stddef.h, for size_t.
- pagealign_alloc.c:
- Remove the mention of public domain since it contradicts the GPL.
- Include pagealign_alloc.h first, to verify that it is self-consistent.
- Use the 'exit' module for EXIT_FAILURE.
- Don't use EINVAL in an error message that is already verbose enough.
- Drop the trailing dot in error messages that use an errno, since
they are displayed with a colon and the errno explanation following it.
- Fixed portability problem of void* computations like 'orig + pagesize -
1'.
- Use a typedef to avoid many #ifs.
- Rename input parameter 'out' to 'aligned_ptr'. (An input parameter called
'out' is somewhat paradox.)
- Don't use 'new' as variable name, since we might want to compile the
code with a C++ compiler some day.
- Removed unnecessary fields from 'memtable' and renamed it to
'memnode_table'.
- In get_memnode, use abort() instead of exit() to signal a bug in the
calling code.
- Make pagealign_alloc work also if a 'void *' does not fit in a 'long'.
- Use #if HAVE_POSIX_MEMALIGN consistently, not a mix of #if here and
#ifdef there.
- mmap.m4: Renamed to mmap-anon.m4 since it cares only about anonymous
mappings, not about file or shared mappings. ('grep' has a different
test for mmap, and 'clisp' yet another one.)
Write config.h, not config.h.
Do you still see some nits that could be improved?
Bruno
=== modules/pagealign_alloc
Description:
Memory allocation aligned on page boundaries.
Files:
lib/pagealign_alloc.h
lib/pagealign_alloc.c
m4/mmap-anon.m4
m4/pagealign_alloc.m4
Depends-on:
error
exit
getpagesize
xalloc
configure.ac:
gl_PAGEALIGN_ALLOC
Makefile.am:
Include:
#include pagealign_alloc.h
License:
GPL
Maintainer:
[EMAIL PROTECTED]
= lib/pagealign_alloc.h
/* Memory allocation aligned to system page boundaries.
Copyright (C) 2005 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify it
under the terms of the GNU General Public License as published
by the Free Software Foundation; either version 2, or (at your option)
any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
General Public License for more details.
You should have received a copy of the GNU General Public
License along with this program; if not, write to the Free Software
Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307,
USA. */
#ifndef _PAGEALIGN_ALLOC_H
# define _PAGEALIGN_ALLOC_H
# include stddef.h
/* Allocate a block of memory of SIZE bytes, aligned on a system page
boundary.
If SIZE is not a multiple of the system page size, it will be rounded up
to the next multiple.
Return a pointer to the start of the memory block, or NULL if the allocation
failed. */
extern void *pagealign_alloc (size_t size);
/* Free a memory block.
PTR must be a pointer returned by pagealign_alloc. */
extern void pagealign_free (void *ptr);
#endif /* _PAGEALIGN_ALLOC_H */
= lib/pagealign_alloc.c
/* Memory allocation aligned to system page boundaries.
Copyright (C) 2005 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify it
under the terms of the GNU General Public License as published
by the Free Software Foundation; either version 2, or (at your option)
any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
General Public License for more details.
You should have received a copy of the GNU General Public
License along with this program; if not, write to the Free Software
Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307,
USA. */
/* Written by Derek R. Price [EMAIL PROTECTED]. */
#ifdef HAVE_CONFIG_H
# include config.h
#endif
#include pagealign_alloc.h
#include errno.h
#include stdlib.h
#if HAVE_FCNTL_H
# include fcntl.h
#endif
#if HAVE_UNISTD_H
# include unistd.h
#endif
#if HAVE_MMAP
# include sys/mman.h
#endif
#include error.h
#include exit.h
#include getpagesize.h
#include xalloc.h
#if
Re: Feature request/ideas
Derek Price wrote: | | So probably the expression used should connote this. After some | | consideration, I would vote for '.origin' here. I disagree | with | being meaningless. I often export a project state into a | local | repository, work on it, and when I'm done, move the files | back to | the remote repository's sandbox. During local | development I often | want to compare to the initial version of a | file, and using a | single tag for this is just easy. Granted | there are other ways to | achieve this, but they're not as easy | to handle. | | That's fine for 1.1, but how does this help you for a branch? | You might want to diff against the root, but it doesn't make much | sense to care about the first revision on the branch. | | | Good point. What about resolving '.origin' to the very first | revision of the mainline? I don't have any reason to object to that. On further consideration, why doesn't -r1.1 suffice for what you want to do? Regards, Derek ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: `cvs up -p FILE' does not detect write failure
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Jim Meyering wrote: | Here's a complete patch, minus the parts that are regenerated via | autoreconf (which I'll check in, too, being careful to use | automake-1.9.3, which is what generated the current Makefile.in | files -- unless we can upgrade to automake-1.9.5 -- quite safe, | IMHO). I generally don't mind keeping as up-to-date as possible with Automake, but I also tend to avoid upping the version we use with CVS more frequently than every few months, just to avoid creating extra work for the other maintainers, unless there is a need for some new feature or a fix for a serious bug. It's been a good 3.5 months since our last AM upgrade, so there shouldn't be much problem going to 1.9.5 now if noone objects. Regards, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCJzHiLD1OTBfyMaQRAqGrAKDBxrsBeWbvsz4Khodanb3fkjf57gCgqV6z COaqSy3XlPZbx6V1qxNb860= =mreV -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: Feature request/ideas
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Derek Price [EMAIL PROTECTED] writes: Derek Price wrote: | | So probably the expression used should connote this. After some | | consideration, I would vote for '.origin' here. I disagree | with | being meaningless. I often export a project state into a | local | repository, work on it, and when I'm done, move the files | back to | the remote repository's sandbox. During local | development I often | want to compare to the initial version of a | file, and using a | single tag for this is just easy. Granted | there are other ways to | achieve this, but they're not as easy | to handle. | | That's fine for 1.1, but how does this help you for a branch? | You might want to diff against the root, but it doesn't make much | sense to care about the first revision on the branch. | | | Good point. What about resolving '.origin' to the very first | revision of the mainline? I don't have any reason to object to that. On further consideration, why doesn't -r1.1 suffice for what you want to do? Possibly for handling the following conditions... - cvs add foo cvs commit -mnew foo echo newstuff foo \ cvs commit -mupdate foo cvs admin -o1.1 foo .origin == 1.2 after this operation - cvs add foo cvs commit -mnew -r2.1 foo .origin == 2.1 - cvs tag -b foo-branch cvs update -rfoo-branch cvs add foo \ cvs commit -mnew foo In this case, is .origin == 1.1 (dead) or is it not found? - cvs tag -b foo-branch cvs update -rfoo-branch cvs add foo \ cvs commit -mnew foo cvs update -A cvs up -jfoo-branch \ cvs commit -mmerge foo .origin == 1.2 I have no objections to .origin being used for the very first revision of the mainline. -- Mark -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.3 (FreeBSD) iD8DBQFCJzbw3x41pRYZE/gRApiNAKC8hOUGy1lvebo27DDnxlXORclf3ACdHabl CAwLIZj9tAonI7HPZGzhEzM= =GtfW -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
Derek Price wrote:
- Rename macro gl_FUNC_MMAP to gl_FUNC_MMAP_ANON to sync
with new m4 file name.
Applied, thanks.
- Remove unnecessary fd set.
You can even optimize away the 'fd' variable entirely in this case and
make it a constant.
- Comment in header says pagealign_alloc() may return NULL. It may not -
it will exit via error() instead.
Ah, good point. Coming to think of it:
- The details of the error messages (mmap to /dev/zero failed,
posix_memalign failed) both boil down to the same, namely
memory exhausted. For an end user, memory exhausted is more
understandable, so let's use this.
- Some programs want to do custom actions when an out-of-memory
situation occurs. That's why we have xalloc_die(). So xalloc_die() should
be called here instead of doing error(EXIT_FAILURE,0,memory exhausted).
- Some programs may want to have a pagealign_alloc() that returns NULL
upon failure. Should pagealign_alloc() therefore return NULL or call
xalloc_die()?
- The answer is clear: We usually use an 'x' in the function name ('x'
means 'checking') when xalloc_die() may be called. Therefore if we
provide a function pagealign_alloc() which returns NULL upon failure
and a function pagealign_xalloc() which calls xalloc_die() in this
case, both needs are covered.
So I committed the appended patch.
And one question:
Why do you cast to char * here, when ret is actually a void *? Is this
necessary?
void *unaligned_ptr = xmalloc (size + pagesize - 1);
ret = (char *) unaligned_ptr
+ ((- (unsigned long) unaligned_ptr) (pagesize - 1));
This is necessary because 'void *' + 'integer' is not defined in ANSI C nor
ISO C99.
Only GCC interprets it the same way as 'char *' + 'integer'; all other
compilers give an error.
Bruno
diff -c -3 -r1.1 pagealign_alloc.h
*** pagealign_alloc.h 3 Mar 2005 14:07:04 - 1.1
--- pagealign_alloc.h 3 Mar 2005 16:19:47 -
***
*** 30,37
failed. */
extern void *pagealign_alloc (size_t size);
/* Free a memory block.
!PTR must be a pointer returned by pagealign_alloc. */
extern void pagealign_free (void *ptr);
#endif /* _PAGEALIGN_ALLOC_H */
--- 30,42
failed. */
extern void *pagealign_alloc (size_t size);
+ /* Like pagealign_alloc, except it exits the program if the allocation
+fails. */
+ extern void *pagealign_xalloc (size_t size);
+
/* Free a memory block.
!PTR must be a non-NULL pointer returned by pagealign_alloc or
!pagealign_xalloc. */
extern void pagealign_free (void *ptr);
#endif /* _PAGEALIGN_ALLOC_H */
diff -c -3 -r1.1 pagealign_alloc.c
*** pagealign_alloc.c 3 Mar 2005 14:07:04 - 1.1
--- pagealign_alloc.c 3 Mar 2005 16:19:47 -
***
*** 117,129
void *ret;
#if HAVE_MMAP
int flags;
- static int fd = -1; /* Only open /dev/zero once in order to avoid limiting
- the amount of memory we may allocate based on the
- number of open file descriptors. */
# ifdef HAVE_MAP_ANONYMOUS
flags = MAP_ANONYMOUS | MAP_PRIVATE;
- fd = -1;
# else /* !HAVE_MAP_ANONYMOUS */
flags = MAP_FILE | MAP_PRIVATE;
if (fd == -1)
{
--- 117,129
void *ret;
#if HAVE_MMAP
int flags;
# ifdef HAVE_MAP_ANONYMOUS
+ const int fd = -1;
flags = MAP_ANONYMOUS | MAP_PRIVATE;
# else /* !HAVE_MAP_ANONYMOUS */
+ static int fd = -1; /* Only open /dev/zero once in order to avoid limiting
+ the amount of memory we may allocate based on the
+ number of open file descriptors. */
flags = MAP_FILE | MAP_PRIVATE;
if (fd == -1)
{
***
*** 134,148
# endif /* HAVE_MAP_ANONYMOUS */
ret = mmap (NULL, size, PROT_READ | PROT_WRITE, flags, fd, 0);
if (!ret)
! error (EXIT_FAILURE, errno, mmap to /dev/zero failed);
new_memnode (ret, size);
#elif HAVE_POSIX_MEMALIGN
int status = posix_memalign (ret, getpagesize (), size);
if (status)
! error (EXIT_FAILURE, status, posix_memalign failed);
#else /* !HAVE_MMAP !HAVE_POSIX_MEMALIGN */
size_t pagesize = getpagesize ();
! void *unaligned_ptr = xmalloc (size + pagesize - 1);
ret = (char *) unaligned_ptr
+ ((- (unsigned long) unaligned_ptr) (pagesize - 1));
new_memnode (ret, unaligned_ptr);
--- 134,150
# endif /* HAVE_MAP_ANONYMOUS */
ret = mmap (NULL, size, PROT_READ | PROT_WRITE, flags, fd, 0);
if (!ret)
! return NULL;
new_memnode (ret, size);
#elif HAVE_POSIX_MEMALIGN
int status = posix_memalign (ret, getpagesize (), size);
if (status)
! return NULL;
#else /* !HAVE_MMAP !HAVE_POSIX_MEMALIGN */
size_t pagesize = getpagesize ();
! void *unaligned_ptr = malloc (size + pagesize - 1);
! if (unaligned_ptr == NULL)
! return NULL;
ret = (char *) unaligned_ptr
+ ((-
RE: lib/xtime.h is included twice
Derek Price wrote: Jim.Hyslop wrote: | [EMAIL PROTECTED] wrote: | | Dear cvs developers, | | for cvs 1.11.19 and earlier, lib/xtime.h is included twice, which | breaks compiling on IRIX 5.3. The following patch prevents this: | | --- lib/xtime.h.orig2005-03-01 15:53:11.0 +0100 +++ | lib/xtime.h 2005-03-01 15:54:20.0 +0100 @@ -12,6 +12,9 @@ | * functions */ | | +#ifndef _XTIME_H_ +#define _XTIME_H_ | | ISO C Standard reserves names beginning with underscore and an | uppercase character, or beginning with two underscores. I recommend | changing this to something like: | | #ifndef XTIME_HEADER_INCLUDEd #define XTIME_HEADER_INCLUDEd | | (note: the lower-case d at the end is deliberate - it's my | technique to reduce the chances of a name collision.) I'm ok with this if you'd like to apply it, Jim. Will do. Georg, have you tested your patch on any other platforms? It should be harmless, but I've been bitten by enough harmless-looking patches to be wary :-) -- Jim Hyslop Senior Software Designer Leitch Technology International Inc. ( http://www.leitch.com ) Columnist, C/C++ Users Journal ( http://www.cuj.com/experts ) ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: Feature request/ideas
Mark D. Baushke writes: I have no objections to .origin being used for the very first revision of the mainline. Why bother with a special name? Just use .trunk.root. -Larry Jones Ever notice how tense grown-ups get when they're recreating? -- Calvin ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
Do you still see some nits that could be improved?
Oops, there was one more nit: The error messages were not internationalized.
I did this:
diff -c -3 -r1.2 pagealign_alloc.c
*** lib/pagealign_alloc.c 3 Mar 2005 16:21:00 - 1.2
--- lib/pagealign_alloc.c 3 Mar 2005 16:36:58 -
***
*** 44,49
--- 44,52
#include exit.h
#include getpagesize.h
#include xalloc.h
+ #include gettext.h
+
+ #define _(str) gettext (str)
#if HAVE_MMAP || ! HAVE_POSIX_MEMALIGN
***
*** 129,135
{
fd = open (/dev/zero, O_RDONLY, 0666);
if (fd 0)
! error (EXIT_FAILURE, errno, Failed to open /dev/zero for read);
}
# endif /* HAVE_MAP_ANONYMOUS */
ret = mmap (NULL, size, PROT_READ | PROT_WRITE, flags, fd, 0);
--- 132,138
{
fd = open (/dev/zero, O_RDONLY, 0666);
if (fd 0)
! error (EXIT_FAILURE, errno, _(Failed to open /dev/zero for read));
}
# endif /* HAVE_MAP_ANONYMOUS */
ret = mmap (NULL, size, PROT_READ | PROT_WRITE, flags, fd, 0);
diff -c -3 -r1.1 pagealign_alloc
*** modules/pagealign_alloc 3 Mar 2005 14:07:04 - 1.1
--- modules/pagealign_alloc 3 Mar 2005 16:36:58 -
***
*** 11,16
--- 11,17
error
exit
getpagesize
+ gettext
xalloc
configure.ac:
___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: Feature request/ideas
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Larry Jones [EMAIL PROTECTED] writes: Mark D. Baushke writes: I have no objections to .origin being used for the very first revision of the mainline. Why bother with a special name? Just use .trunk.root. Hmmm... true enough. The .origin modifier is not needed. -- Mark -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.3 (FreeBSD) iD8DBQFCJz7i3x41pRYZE/gRAhGOAKCTH43+MaleWMGCDnQnqaB8hkENogCeIlxy MZp0E7grYWDn84dUoMuPUK4= =Tizu -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: Feature request/ideas
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Mark D. Baushke wrote: | Derek Price [EMAIL PROTECTED] writes: | | Derek Price wrote: | | | | So probably the expression used should connote this. After | some | | consideration, I would vote for '.origin' here. I | disagree | with | being meaningless. I often export a project | state into a | local | repository, work on it, and when I'm | done, move the files | back to | the remote repository's | sandbox. During local | development I often | want to compare | to the initial version of a | file, and using a | single tag | for this is just easy. Granted | there are other ways to | | achieve this, but they're not as easy | to handle. | | | That's fine for 1.1, but how does this help you for a branch? | | You might want to diff against the root, but it doesn't make | much | sense to care about the first revision on the branch. | | | | Good point. What about resolving '.origin' to the very | first | revision of the mainline? | | | I don't have any reason to object to that. | | | On further consideration, why doesn't -r1.1 suffice for what you | want to do? | | | Possibly for handling the following conditions... | | - cvs add foo cvs commit -mnew foo echo newstuff foo \ | cvs commit -mupdate foo cvs admin -o1.1 foo | | .origin == 1.2 after this operation | | - cvs add foo cvs commit -mnew -r2.1 foo | | .origin == 2.1 Well, yes, but -r1.2 or -r2.1 would suffice in these cases, though I will grant you have to know what revision to use. I would hazard that there is either a -rrev revision that can be specified here across multiple files or that the result of a multi-file .origin will likely be meaningless anyhow. In the specific example Frank specified, also, -r1.1 should always work. | - cvs tag -b foo-branch cvs update -rfoo-branch cvs add foo \ | cvs commit -mnew foo | | In this case, is .origin == 1.1 (dead) or is it not found? I have no idea. I think for most use cases either will have the same result. For cvs up -r.origin foo, where foo has no origin, I see little difference between an error message that says .origin not found and a silent checkout of nothing (the dead revision), but maybe someone else has a reason to prefer one over the other. | - cvs tag -b foo-branch cvs update -rfoo-branch cvs add foo \ | cvs commit -mnew foo cvs update -A cvs up -jfoo-branch \ | cvs commit -mmerge foo | | .origin == 1.2 I don't think so. This should be consistent with the answer to your previous question. If the -r.origin with only a dead revision returns the dead revision, then this .origin should also return it. If - -r.origin with only a dead revision returns no revision, then this should return 1.2, as you specify. | I have no objections to .origin being used for the very first | revision of the mainline. It's not that big a deal, really, but I would like to hear a use case that can't be satisified with a simple revision selection or hear a person or two declare strongly that they prefer the convenience of an .origin that may sometimes be meaningless to an additional call or two to `cvs log'. Regards, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCJ0A8LD1OTBfyMaQRAsjPAKDAJODmsV7MYXc+LJ/eQ6wI9XDPmgCg257L Ntyx7xsNC6o6XAr15UT/Yrw= =UzxB -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: Feature request/ideas
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Mark D. Baushke wrote: | Larry Jones [EMAIL PROTECTED] writes: | | Mark D. Baushke writes: | | I have no objections to .origin being used for the very first | revision of the mainline. | | Why bother with a special name? Just use .trunk.root. | | | Hmmm... true enough. The .origin modifier is not needed. I thought Mark was just saying earlier in this thread that .trunk.root, by virtue of .root normally specifying a revision on the parent branch, should refer to the `0' revision? Regards, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCJ0IOLD1OTBfyMaQRAj6rAJ9gZI9mNMCT5aukpgDjh/4u6i1VyQCgloFJ /R17eO0mATKq3GlWD8WcXw8= =rKbJ -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: Feature request/ideas
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Derek Price [EMAIL PROTECTED] writes: I thought Mark was just saying earlier in this thread that .trunk.root, by virtue of .root normally specifying a revision on the parent branch, should refer to the `0' revision? Hmmm... I may be getting confused, I thought .trunk.root.prev would be the `0' revision, but if you are right then .trunk.root.next would be the same as .origin right? As for `0', we probably need to be more consistent about it... for example, the command: cvs rdiff -r0 -r1.1 foo-module/foo-file yeilds a diff against /dev/null of the foo-file while the commands cvs co foo-module cd foo-module cvs diff -r0 -r1.1 foo-file yeilds an error message: 'cvs diff: tag 0 is not in file foo-file' this probably needs to be `fixed' to do something reasonable with `0' for most uses as a phantom revision that exists before the file existed on the branch or trunk. -- Mark -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.3 (FreeBSD) iD8DBQFCJ0QA3x41pRYZE/gRAtxLAKDKCpFooWEpuPrQ+QKaZFWKntp8jwCgg/st m5sIT/JB03xZC4SdQDnRBYk= =R37G -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: Feature request/ideas
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Mark D. Baushke wrote: | Derek Price [EMAIL PROTECTED] writes: | | I thought Mark was just saying earlier in this thread that | .trunk.root, by virtue of .root normally specifying a revision on | the parent branch, should refer to the `0' revision? | | | Hmmm... I may be getting confused, I thought .trunk.root.prev would | be the `0' revision, but if you are right then .trunk.root.next | would be the same as .origin right? No. I think 0.next would be an invalid construct, or also return 0. If you take this off the trunk, this might make more sense: BRANCH.root is on the trunk (or another branch), so BRANCH.root.next would return the revision following the root revision on the parent. For example, 1.2.2.7.root would return 1.2. Since 1.2.next yields 1.3, then 1.2.2.7.root.next should also yield 1.3. Since there is no revision following on the `0 branch', .trunk.root.next should either also be 0 or be invalid. | As for `0', we probably need to be more consistent about it... for | example, the command: | | cvs rdiff -r0 -r1.1 foo-module/foo-file | | yeilds a diff against /dev/null of the foo-file while the commands | | cvs co foo-module cd foo-module cvs diff -r0 -r1.1 foo-file | | yeilds an error message: | | 'cvs diff: tag 0 is not in file foo-file' | | this probably needs to be `fixed' to do something reasonable with | `0' for most uses as a phantom revision that exists before the file | existed on the branch or trunk. I agree. Regards, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCJ0c2LD1OTBfyMaQRAj0dAJ9Qt/gkL2Vunx6y6A0GB4qxF+rqCwCgjrtr EnCQIIQjVKK4dNM8unXhwAY= =3dZx -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Bruno Haible wrote:
| Do you still see some nits that could be improved?
|
|
| Oops, there was one more nit: The error messages were not
| internationalized. I did this:
[snip]
| *** *** 129,135 { fd = open (/dev/zero,
| O_RDONLY, 0666); if (fd 0) ! error (EXIT_FAILURE, errno, Failed
| to open /dev/zero for read); } # endif /* HAVE_MAP_ANONYMOUS */
| ret = mmap (NULL, size, PROT_READ | PROT_WRITE, flags, fd, 0); ---
| 132,138 { fd = open (/dev/zero, O_RDONLY, 0666); if (fd 0)
| ! error (EXIT_FAILURE, errno, _(Failed to open /dev/zero for
| read)); } # endif /* HAVE_MAP_ANONYMOUS */ ret = mmap (NULL, size,
| PROT_READ | PROT_WRITE, flags, fd, 0);
Should this error still be exiting (EXIT_FAILURE) when this function
is defined to return NULL on failure?
Regards,
Derek
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFCJ0j/LD1OTBfyMaQRAtohAJ9eNQ/a2dO1wXrfdw4WKfztB5UTzwCgjPdP
F3Y6HTeZBnfMFD6SZ5AdOqQ=
=zhqn
-END PGP SIGNATURE-
___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Re my earlier questions about efficiency, here's the patch using the new pagealign_alloc module which is currently passing `make remotecheck' here. It also contains a few modification to the new GNULIB pagealign_alloc module because I haven't finalized them and submitted them to bug-gnulib yet, but I should shortly. Comments or benchmarks are appreciated. Regards, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCJ0qWLD1OTBfyMaQRAg15AJ0X15wNNTvYvL7i+yVr9lkycBGlkgCg0wqw bbMrUmS9W1T3vBWiXIJDLA0= =hynq -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: Feature request/ideas
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Derek Price [EMAIL PROTECTED] writes: No. I think 0.next would be an invalid construct, or also return 0. Yeah, you are correct. If you take this off the trunk, this might make more sense: BRANCH.root is on the trunk (or another branch), so BRANCH.root.next would return the revision following the root revision on the parent. For example, 1.2.2.7.root would return 1.2. Since 1.2.next yields 1.3, then 1.2.2.7.root.next should also yield 1.3. Yup. Since there is no revision following on the `0 branch', .trunk.root.next should either also be 0 or be invalid. Agreed. Given that 1.2.2.7.root == 1.2 which is the predicessor revision to the first revision on the branch, and .trunk being on the TRUNK, then .trunk.root is the predicessor revision for the TRUNK also known as `0'. Therefore, I suppose that there could be a need for .origin to be the first revision on TRUNK and .trunk.head to replace HEAD on TRUNK. Looking at a mixture of the modifiers with regard to time... One presumes that '.trunk:2005-03-01 08:00:00 UTC' would be the revision that was committed just before 2005-03-01 08:00:00 UTC. It is less clear how one would specify the .next revision on the TRUNK for that case... -- Mark -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.3 (FreeBSD) iD8DBQFCJ1uH3x41pRYZE/gRAsoZAKCfPuDJHWrt+y3Qtwk2AfGe9inw1ACgyQub /m83ZvvHmEFzVQtDX8fo78k= =2HUw -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: Feature request/ideas
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Mark D. Baushke wrote: | Therefore, I suppose that there could be a need for .origin to be | the first revision on TRUNK This would seldom mean much across multiple files, so I still think .origin should not be used. The case Frank cited, where he is basically trying to diff against an import (thought not generated using the import command), is the only one where all the .origin revisions will be related in a sensible way, and even then only if no files have been added or removed on the trunk. Once files have been added or removed, you degenerate to the case where the .origin revisions (or even 1.1 revisions) of these files could have been added at different times and offering to calculate .origin is misleading at best. The only consistent way to do this is to tag everything after the import and diff against that tag. This tag couldn't even really be automated, except in something like the import command, which imports a set of files at once and tags the set. .origin makes no sense. | Looking at a mixture of the modifiers with regard to time... | | One presumes that '.trunk:2005-03-01 08:00:00 UTC' would be the | revision that was committed just before 2005-03-01 08:00:00 UTC. It | is less clear how one would specify the .next revision on the TRUNK | for that case... As this stands, with : and . already overloaded in time specs, this could get complex, but I will grant that the feature might be nice once the syntax was ironed out. Regards, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCJ2InLD1OTBfyMaQRAl2tAJ9e8+0pKMDCVkgBBpoSE75JKMZq2QCgrrAB ab9OQ+bJzuJmZMOaqDjOCCo= =7Iy6 -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Bruno Haible wrote: | Derek Price wrote: | | Should this error still be exiting (EXIT_FAILURE) when this | function is defined to return NULL on failure? | | | In this case the error is not memory exhausted, it is cannot | open /dev/zero. This can happen for example if someone is working | in a chroot environment that is incorrectly set up. I think a clear | error message is good in this case. But I agree it's hard to say in | advance which behaviour is better. It could print the error message with error (0, errno, ...) and then return NULL. The caller could then decide if that error should be fatal or not, as I presume they might wish to if they are calling pagealign_alloc() as opposed to pagealign_xalloc(). Regards, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCJ188LD1OTBfyMaQRAisLAKCjzfZm7V5uPkRIXQ3ILJSLhHIHRwCgkNSz qL4MjglKCWi/7OEocliSR28= =ubwl -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: `cvs up -p FILE' does not detect write failure
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Derek Price [EMAIL PROTECTED] writes: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Jim Meyering wrote: | Here's a complete patch, minus the parts that are regenerated via | autoreconf (which I'll check in, too, being careful to use | automake-1.9.3, which is what generated the current Makefile.in | files -- unless we can upgrade to automake-1.9.5 -- quite safe, | IMHO). I generally don't mind keeping as up-to-date as possible with Automake, but I also tend to avoid upping the version we use with CVS more frequently than every few months, just to avoid creating extra work for the other maintainers, unless there is a need for some new feature or a fix for a serious bug. It's been a good 3.5 months since our last AM upgrade, so there shouldn't be much problem going to 1.9.5 now if noone objects. I have no objections to moving to automake-1.9.5. -- Mark -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.3 (FreeBSD) iD8DBQFCJ2Gk3x41pRYZE/gRAly3AKCSYizNa8azRgQi2pMT7A5WZavXpACgvQWx 8tOWfrs/nm6Qdzt7GAZJimc= =Xtm8 -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
Derek Price wrote: It could print the error message with error (0, errno, ...) and then return NULL. The caller could then decide if that error should be fatal or not, as I presume they might wish to if they are calling pagealign_alloc() as opposed to pagealign_xalloc(). Maybe... but it makes things more complicated for the caller. And if this error occurs, it's a problem in the environment, not in the program's data; and since it occurs upon the first call to pagealign_alloc, you can assume that the program does not have sensitive data to save. Bruno ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
GNULIB wait-process module.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hello. I was looking at removing CVS's internal waitpid substitute in favor of the GNULIB wait-process module, but it's missing one feature I need. CVS needs to detect core dumps in the child of the server so that the parent knows not to clean up the temporary workspace the server is using. This is for debugging purposes, really, but I can think of no good alternative that doesn't require a complete . I could stick with waitpid() in this instance, but I would like to include the other features of wait-process and waitpid() is the last AC_REPLACE_FUNC module in our lib directory that doesn't come from GNULIB. (Well, valloc() is still technically in there, but I believe that I have almost eliminated that re the recent thread.) Would you be amenable to a modification to wait-process that allows core dumps to be detected? I would be satisfied if a 126 return status meant a core dump, but the most general solution might be an extra optional signal var passed in that was filled with sanitized signal codes (e.g. SIGXXX) when such were detected. Regards, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCJ3kCLD1OTBfyMaQRAhsXAJ42yJxZ3QIxqfreGmraRv9uFEbERACgk+xR DVZF65S39zr42gqGPbVKI2o= =OXbb -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
Derek Price wrote: I've attached a patch to fix a few more nits Committed, with small wording tweaks in the comments. - I noticed that I assumed mmap() returns NULL on error when it actually returns MAP_FAILED according to POSIX. Ouch, this was a major bug. Glad that you found it. - Note in the header that pagealign_alloc sets errno on failure. But it doesn't do so if malloc() fails on non-POSIX systems (like mingw or so). I think one should set errno = ENOMEM if malloc() fails. I had two further thoughts on the naming of mmap-anon.m4. First, in my original decision to name it mmap.m4, I did take into consideration that there might be other mmap.m4 implementations but, ideally, I would hope that their requirements could be merged into this mmap(-anon).m4, at the least for simplicity's sake. I don't have this hope: The requirements of different programs regarding mmap are so different that, if all tests were merged into a common mmap.m4 file, some people would say this test is insane - it disables mmap on SVR4 [or Linux 1.2 or HP-UX or ...] although it is perfectly sane. SVR4 had problems with PRIVATE READ-WRITE mappings of files. Linux 1.2 didn't support MAP_SHARED on files. HP-UX doesn't support mapping files at fixed addresses in most cases. Second, technically what we are currently calling gl_FUNC_MMAP_ANON calls AC_FUNC_MMAP, so it also verifies that private, fixed maps to files work as well, not just anonymous maps. Yes, this is one of the problems with AC_FUNC_MMAP. But fortunately we can ignore it because nowadays most systems have a working mmap(), therefore not many people will complain it disables mmap() on my system. Bruno ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] valloc()?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Bruno Haible wrote: | Derek Price wrote: | | It could print the error message with error (0, errno, ...) and | then return NULL. The caller could then decide if that error | should be fatal or not, as I presume they might wish to if they | are calling pagealign_alloc() as opposed to pagealign_xalloc(). | | | Maybe... but it makes things more complicated for the caller. And | if this error occurs, it's a problem in the environment, not in the | program's data; and since it occurs upon the first call to | pagealign_alloc, you can assume that the program does not have | sensitive data to save. Good point. I'll concede that. Regards, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCJ3lwLD1OTBfyMaQRAks/AJwMOHm3oSDKrg/lvQ13Y6Urwu4C+QCg6No/ a6xUjZzVmVfqLKmIGam5Qso= =MgCn -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
