Created attachment 9058 [details] patch file for shared_mmap.c in /lib/replace/test Hello All, In reviewing source code files in Samba-4.0.7, I found several instances where the library call 'mmap()' is not checked for a return value of (int *)-1, indicating failure. In directory 'samba-4.0.7/lib/replace/test', file 'shared_mmap.c', I found an instance where mmap() was called without a test for failure. The patch file below adds the test: --- shared_mmap.c.orig 2013-07-16 15:58:02.949981134 -0700 +++ shared_mmap.c 2013-07-16 15:58:30.811980273 -0700 @@ -39,6 +39,8 @@ MAP_FILE | MAP_SHARED, fd, 0); + if (buf == (int *)-1) exit(1); + while (count-- && buf[9124] != 55732) sleep(1); if (count <= 0) exit(1); I am attaching the patch file to this bug report Bill Parker (wp02855 at gmail dot com)
Shouldn't the -1 be replaced by MAP_FAILED?
Comment on attachment 9058 [details] patch file for shared_mmap.c in /lib/replace/test besides that, the patch does not comply to README.Coding.. it uses multiple statements on one line and no braces after the if statement
(In reply to comment #1) > Shouldn't the -1 be replaced by MAP_FAILED? That is a mistake on my part, but further down in the same file, the -1 is used rather than MAP_FAILED (inconsistent).
(In reply to comment #2) > Comment on attachment 9058 [details] > patch file for shared_mmap.c in /lib/replace/test > > besides that, the patch does not comply to README.Coding.. it uses multiple > statements on one line and no braces after the if statement Does this not also apply to the existing lines in shared_mmap.c below: while (count-- && buf[9124] != 55732) sleep(1); if (count <= 0) exit(1); I don't really see the difference between the if (count <=0) exit(1); and if (buf == (int *)-1) exit(1); right?
Much code in samba does not comply with the most modern README.Coding rules, but we try to make progress by improving the code as we change it. A second patch to fix up other clear violations in would be much appreciated.
Do you want me to go through the entire file for which the previous mmap() patch was submitted and fix it all? I can do this, but it will take some time. On Tue, Jul 23, 2013 at 12:41 PM, <samba-bugs@samba.org> wrote: > https://bugzilla.samba.org/show_bug.cgi?id=10033 > > --- Comment #5 from Andrew Bartlett <abartlet@samba.org> 2013-07-23 > 19:41:54 UTC --- > Much code in samba does not comply with the most modern README.Coding > rules, > but we try to make progress by improving the code as we change it. A > second > patch to fix up other clear violations in would be much appreciated. > > -- > Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email > ------- You are receiving this mail because: ------- > You reported the bug. >
Created attachment 9077 [details] Revised patch for shared_mmap.c This patch file now corresponds to the README.Coding file in Samba-4.0.7.
Created attachment 9078 [details] Corrects file for README.Coding standard This patch file converts incoherent_mmap.c so that it follows coding standards in README.Coding
Created attachment 9079 [details] Corrects file for README.Coding standard This patch file converts os2_delete.c so that it follows coding standards in README.Coding
Created attachment 9080 [details] Corrects file for README.Coding standard This patch file converts snprintf.c so that it follows coding standards in README.Coding
Created attachment 9081 [details] Corrects file for README.Coding standard This patch file converts strptime.c so that it follows coding standards in README.Coding