Re: mmap diff for patch(1)
On Fri, Nov 11, 2011 at 03:04:28PM +0800, Michael W. Bombardieri wrote:
Sorry, let me try that again... Forgot to clean up file descriptor ifd.
AFAIK, this works without this diff. What problem are you trying to solve?
-Otto
Index: inp.c
===
RCS file: /usr/src/cvs/src/usr.bin/patch/inp.c,v
retrieving revision 1.35
diff -u -r1.35 inp.c
--- inp.c 27 Oct 2009 23:59:41 - 1.35
+++ inp.c 11 Nov 2011 06:51:13 -
@@ -243,6 +243,10 @@
if ((ifd = open(filename, O_RDONLY)) 0)
pfatal(can't open file %s, filename);
+ if (i_size == 0) {
+ close(ifd);
+ return true;
+ }
i_womp = mmap(NULL, i_size, PROT_READ, MAP_PRIVATE, ifd, 0);
if (i_womp == MAP_FAILED) {
perror(mmap failed);
Re: mmap diff for patch(1)
On Fri, Nov 11, 2011 at 10:20:19AM +0100, Otto Moerbeek wrote: On Fri, Nov 11, 2011 at 03:04:28PM +0800, Michael W. Bombardieri wrote: Sorry, let me try that again... Forgot to clean up file descriptor ifd. AFAIK, this works without this diff. What problem are you trying to solve? Some systems reject mmap with size 0. That's what the original chance was all about. Joerg
Re: mmap diff for patch(1)
I don't see any reason to even open the file if the size is 0.
On Fri, Nov 11, 2011, Michael W. Bombardieri wrote:
Sorry, let me try that again... Forgot to clean up file descriptor ifd.
Index: inp.c
===
RCS file: /usr/src/cvs/src/usr.bin/patch/inp.c,v
retrieving revision 1.35
diff -u -r1.35 inp.c
--- inp.c 27 Oct 2009 23:59:41 - 1.35
+++ inp.c 11 Nov 2011 06:51:13 -
@@ -243,6 +243,10 @@
if ((ifd = open(filename, O_RDONLY)) 0)
pfatal(can't open file %s, filename);
+ if (i_size == 0) {
+ close(ifd);
+ return true;
+ }
i_womp = mmap(NULL, i_size, PROT_READ, MAP_PRIVATE, ifd, 0);
if (i_womp == MAP_FAILED) {
perror(mmap failed);
mmap diff for patch(1)
Hi tech,
I have a diff for the patch(1) tool which copies
one included in NetBSD two years ago...
Source: NetBSD src/usr.bin/patch/inp.c revision 1.23
Purpose: Don't bother mmap'ing an empty file
Comments/OK?
- Michael
Index: inp.c
===
RCS file: /usr/src/cvs/src/usr.bin/patch/inp.c,v
retrieving revision 1.35
diff -u -r1.35 inp.c
--- inp.c 27 Oct 2009 23:59:41 - 1.35
+++ inp.c 11 Nov 2011 03:12:10 -
@@ -243,12 +243,16 @@
if ((ifd = open(filename, O_RDONLY)) 0)
pfatal(can't open file %s, filename);
- i_womp = mmap(NULL, i_size, PROT_READ, MAP_PRIVATE, ifd, 0);
- if (i_womp == MAP_FAILED) {
- perror(mmap failed);
+ if (i_size == 0)
i_womp = NULL;
- close(ifd);
- return false;
+ else {
+ i_womp = mmap(NULL, i_size, PROT_READ, MAP_PRIVATE, ifd, 0);
+ if (i_womp == MAP_FAILED) {
+ perror(mmap failed);
+ i_womp = NULL;
+ close(ifd);
+ return false;
+ }
}
close(ifd);
Re: mmap diff for patch(1)
On Thu, Nov 10, 2011 at 9:26 PM, Michael W. Bombardieri m...@ii.net wrote:
Hi tech,
I have a diff for the patch(1) tool which copies
one included in NetBSD two years ago...
Source: NetBSD src/usr.bin/patch/inp.c revision 1.23
Purpose: Don't bother mmap'ing an empty file
Comments/OK?
- Michael
Index: inp.c
===
RCS file: /usr/src/cvs/src/usr.bin/patch/inp.c,v
retrieving revision 1.35
diff -u -r1.35 inp.c
--- inp.c B B B 27 Oct 2009 23:59:41 - B B B 1.35
+++ inp.c B B B 11 Nov 2011 03:12:10 -
@@ -243,12 +243,16 @@
B B B B if ((ifd = open(filename, O_RDONLY)) 0)
B B B B B B B B pfatal(can't open file %s, filename);
- B B B i_womp = mmap(NULL, i_size, PROT_READ, MAP_PRIVATE, ifd, 0);
- B B B if (i_womp == MAP_FAILED) {
- B B B B B B B perror(mmap failed);
+ B B B if (i_size == 0)
B B B B B B B B i_womp = NULL;
- B B B B B B B close(ifd);
- B B B B B B B return false;
+ B B B else {
+ B B B B B B B i_womp = mmap(NULL, i_size, PROT_READ, MAP_PRIVATE,
ifd, 0);
+ B B B B B B B if (i_womp == MAP_FAILED) {
+ B B B B B B B B B B B perror(mmap failed);
+ B B B B B B B B B B B i_womp = NULL;
+ B B B B B B B B B B B close(ifd);
+ B B B B B B B B B B B return false;
+ B B B B B B B }
B B B B }
B B B B close(ifd);
if if (i_size == 0) {...} returns, there's still need for an else block?...
Just asking...
Re: mmap diff for patch(1)
On Thu, Nov 10, 2011 at 11:10:34PM -0600, Abel Abraham Camarillo Ojeda wrote:
On Thu, Nov 10, 2011 at 9:26 PM, Michael W. Bombardieri m...@ii.net wrote:
Hi tech,
I have a diff for the patch(1) tool which copies
one included in NetBSD two years ago...
Source: NetBSD src/usr.bin/patch/inp.c revision 1.23
Purpose: Don't bother mmap'ing an empty file
Comments/OK?
- Michael
Index: inp.c
===
RCS file: /usr/src/cvs/src/usr.bin/patch/inp.c,v
retrieving revision 1.35
diff -u -r1.35 inp.c
--- inp.c ?? ?? ?? 27 Oct 2009 23:59:41 - ?? ?? ??1.35
+++ inp.c ?? ?? ?? 11 Nov 2011 03:12:10 -
@@ -243,12 +243,16 @@
?? ?? ?? ??if ((ifd = open(filename, O_RDONLY)) 0)
?? ?? ?? ?? ?? ?? ?? ??pfatal(can't open file %s, filename);
- ?? ?? ?? i_womp = mmap(NULL, i_size, PROT_READ, MAP_PRIVATE, ifd, 0);
- ?? ?? ?? if (i_womp == MAP_FAILED) {
- ?? ?? ?? ?? ?? ?? ?? perror(mmap failed);
+ ?? ?? ?? if (i_size == 0)
?? ?? ?? ?? ?? ?? ?? ??i_womp = NULL;
- ?? ?? ?? ?? ?? ?? ?? close(ifd);
- ?? ?? ?? ?? ?? ?? ?? return false;
+ ?? ?? ?? else {
+ ?? ?? ?? ?? ?? ?? ?? i_womp = mmap(NULL, i_size, PROT_READ, MAP_PRIVATE,
ifd, 0);
+ ?? ?? ?? ?? ?? ?? ?? if (i_womp == MAP_FAILED) {
+ ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? perror(mmap failed);
+ ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? i_womp = NULL;
+ ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? close(ifd);
+ ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? return false;
+ ?? ?? ?? ?? ?? ?? ?? }
?? ?? ?? ??}
?? ?? ?? ??close(ifd);
if if (i_size == 0) {...} returns, there's still need for an else block?...
Just asking...
Thanks for your comment.
From what I can see, plan_a() should return true in any case when we get
an empty file, so it should be fairly safe to return directly here as
you mentioned.
I have tested the following diff on i386; it appears to produce the same
behaviour as my previous diff. If this works for you, I'm happy to go
with the simpler diff rather than blindly following the NetBSD code.
- Michael
Index: inp.c
===
RCS file: /usr/src/cvs/src/usr.bin/patch/inp.c,v
retrieving revision 1.35
diff -u -r1.35 inp.c
--- inp.c 27 Oct 2009 23:59:41 - 1.35
+++ inp.c 11 Nov 2011 05:27:41 -
@@ -243,6 +243,8 @@
if ((ifd = open(filename, O_RDONLY)) 0)
pfatal(can't open file %s, filename);
+ if (i_size == 0)
+ return true;
i_womp = mmap(NULL, i_size, PROT_READ, MAP_PRIVATE, ifd, 0);
if (i_womp == MAP_FAILED) {
perror(mmap failed);
Re: mmap diff for patch(1)
Sorry, let me try that again... Forgot to clean up file descriptor ifd.
Index: inp.c
===
RCS file: /usr/src/cvs/src/usr.bin/patch/inp.c,v
retrieving revision 1.35
diff -u -r1.35 inp.c
--- inp.c 27 Oct 2009 23:59:41 - 1.35
+++ inp.c 11 Nov 2011 06:51:13 -
@@ -243,6 +243,10 @@
if ((ifd = open(filename, O_RDONLY)) 0)
pfatal(can't open file %s, filename);
+ if (i_size == 0) {
+ close(ifd);
+ return true;
+ }
i_womp = mmap(NULL, i_size, PROT_READ, MAP_PRIVATE, ifd, 0);
if (i_womp == MAP_FAILED) {
perror(mmap failed);
