Re: svn commit: r196558 - head/usr.bin/look

2009-08-31 Thread John Baldwin
On Saturday 29 August 2009 4:35:16 pm Alan Cox wrote:
 John Baldwin wrote:
  On Tuesday 25 August 2009 11:30:06 pm Colin Percival wrote:

  Author: cperciva
  Date: Wed Aug 26 03:30:06 2009
  New Revision: 196558
  URL: http://svn.freebsd.org/changeset/base/196558
 
  Log:
Don't try to mmap the contents of empty files.  This behaviour was 
  harmless
prior to r195693, since historical behaviour of mmap(2) was to silently
ignore length-zero mmap requests; but mmap now returns EINVAL, which 
  caused
look(1) to emit an error message and fail.
  
 
  FWIW, it did not silently ignore the request.  Instead it rounded the size 
  up
  to a page and mapped a page of data.  However, if you then passed that 
  pointer
  with a length of 0 to munmap() munmap() would fail.
 

 
 I don't believe that your claim is correct; round_page(0) == 0.  Thus, 
 the value of size that would be passed to vm_mmap() would be 0, which 
 would cause it to immediately return 0, indicating success.  In this 
 case, I believe that the virtual address that would be returned by 
 mmap(2) would always be the hint address for where it should start 
 looking for free space, which would be the end of the heap/data segment, 
 unless the application specified its own hint.
 
 In short, and the reason why I'm correcting you here, is to make clear 
 that we needn't worry about earlier versions of FreeBSD that don't 
 implement this change leaking or wasting memory from misuse of mmap(2) 
 in this way.

Yes, I think you are correct.

-- 
John Baldwin
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


Re: svn commit: r196558 - head/usr.bin/look

2009-08-29 Thread Alan Cox

John Baldwin wrote:

On Tuesday 25 August 2009 11:30:06 pm Colin Percival wrote:
  

Author: cperciva
Date: Wed Aug 26 03:30:06 2009
New Revision: 196558
URL: http://svn.freebsd.org/changeset/base/196558

Log:
  Don't try to mmap the contents of empty files.  This behaviour was harmless
  prior to r195693, since historical behaviour of mmap(2) was to silently
  ignore length-zero mmap requests; but mmap now returns EINVAL, which caused
  look(1) to emit an error message and fail.



FWIW, it did not silently ignore the request.  Instead it rounded the size up
to a page and mapped a page of data.  However, if you then passed that pointer
with a length of 0 to munmap() munmap() would fail.

  


I don't believe that your claim is correct; round_page(0) == 0.  Thus, 
the value of size that would be passed to vm_mmap() would be 0, which 
would cause it to immediately return 0, indicating success.  In this 
case, I believe that the virtual address that would be returned by 
mmap(2) would always be the hint address for where it should start 
looking for free space, which would be the end of the heap/data segment, 
unless the application specified its own hint.


In short, and the reason why I'm correcting you here, is to make clear 
that we needn't worry about earlier versions of FreeBSD that don't 
implement this change leaking or wasting memory from misuse of mmap(2) 
in this way.


Alan

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


Re: svn commit: r196558 - head/usr.bin/look

2009-08-26 Thread John Baldwin
On Tuesday 25 August 2009 11:30:06 pm Colin Percival wrote:
 Author: cperciva
 Date: Wed Aug 26 03:30:06 2009
 New Revision: 196558
 URL: http://svn.freebsd.org/changeset/base/196558
 
 Log:
   Don't try to mmap the contents of empty files.  This behaviour was harmless
   prior to r195693, since historical behaviour of mmap(2) was to silently
   ignore length-zero mmap requests; but mmap now returns EINVAL, which caused
   look(1) to emit an error message and fail.

FWIW, it did not silently ignore the request.  Instead it rounded the size up
to a page and mapped a page of data.  However, if you then passed that pointer
with a length of 0 to munmap() munmap() would fail.

-- 
John Baldwin
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


svn commit: r196558 - head/usr.bin/look

2009-08-25 Thread Colin Percival
Author: cperciva
Date: Wed Aug 26 03:30:06 2009
New Revision: 196558
URL: http://svn.freebsd.org/changeset/base/196558

Log:
  Don't try to mmap the contents of empty files.  This behaviour was harmless
  prior to r195693, since historical behaviour of mmap(2) was to silently
  ignore length-zero mmap requests; but mmap now returns EINVAL, which caused
  look(1) to emit an error message and fail.
  
  Among other things, this makes `freebsd-update fetch` on a newly installed
  8.0-BETA3 system print bogus warning messages.
  
  MFC after:3 days

Modified:
  head/usr.bin/look/look.c

Modified: head/usr.bin/look/look.c
==
--- head/usr.bin/look/look.cTue Aug 25 21:51:47 2009(r196557)
+++ head/usr.bin/look/look.cWed Aug 26 03:30:06 2009(r196558)
@@ -140,6 +140,10 @@ main(int argc, char *argv[])
err(2, %s, file);
if (sb.st_size  SIZE_T_MAX)
errx(2, %s: %s, file, strerror(EFBIG));
+   if (sb.st_size == 0) {
+   close(fd);
+   continue;
+   }
if ((front = mmap(NULL, (size_t)sb.st_size, PROT_READ, 
MAP_SHARED, fd, (off_t)0)) == MAP_FAILED)
err(2, %s, file);
back = front + sb.st_size;
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org