Re: [PATCH 9] installboot: malloc/memset => calloc
Commited. Thanks. On Sun, 1 Jun 2014, Benjamin Baier wrote: > On Sun, 1 Jun 2014 00:57:43 +1000 > > Joel Sing wrote: > > In this case I think readability wins. I do not believe that there is a > > lot to gain from overflow protection given the numbers used in these > > calculations are very small (and many are already bounds checked in some > > form or other). > > Well, I had to ask. Here is option 2. > > Index: bootstrap.c > === > RCS file: /cvs/src/usr.sbin/installboot/bootstrap.c,v > retrieving revision 1.3 > diff -u -p -r1.3 bootstrap.c > --- bootstrap.c 28 Dec 2013 12:01:33 - 1.3 > +++ bootstrap.c 31 May 2014 18:14:56 - > @@ -68,10 +68,9 @@ bootstrap(int devfd, char *dev, char *bo > fprintf(stderr, "bootstrap is %zu bytes " > "(%zu sectors @ %u bytes = %zu bytes)\n", > (ssize_t)sb.st_size, bootsec, dl.d_secsize, bootsize); > - boot = malloc(bootsize); > + boot = calloc(1, bootsize); > if (boot == NULL) > - err(1, "malloc"); > - memset(boot, 0, bootsize); > + err(1, "calloc"); > if (read(fd, boot, bootsize) != (ssize_t)sb.st_size) > err(1, "read"); > close(fd); > Index: i386_softraid.c > === > RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v > retrieving revision 1.1 > diff -u -p -r1.1 i386_softraid.c > --- i386_softraid.c 19 Jan 2014 02:58:50 - 1.1 > +++ i386_softraid.c 31 May 2014 18:14:56 - > @@ -129,11 +129,10 @@ sr_install_bootldr(int devfd, char *dev) > inodeblk = nblocks - 1; > bootsize = nblocks * SR_FS_BLOCKSIZE; > > - p = malloc(bootsize); > + p = calloc(1, bootsize); > if (p == NULL) > err(1, NULL); > > - memset(p, 0, bootsize); > fd = open(stage2, O_RDONLY, 0); > if (fd == -1) > err(1, NULL); > Index: sparc64_installboot.c > === > RCS file: /cvs/src/usr.sbin/installboot/sparc64_installboot.c,v > retrieving revision 1.1 > diff -u -p -r1.1 sparc64_installboot.c > --- sparc64_installboot.c 19 Jan 2014 02:58:50 - 1.1 > +++ sparc64_installboot.c 31 May 2014 18:14:56 - > @@ -68,10 +68,9 @@ md_loadboot(void) > if (blksize > SBSIZE - DEV_BSIZE) > errx(1, "boot blocks too big (%zu > %d)", > blksize, SBSIZE - DEV_BSIZE); > - blkstore = malloc(blksize); > + blkstore = calloc(1, blksize); > if (blkstore == NULL) > - err(1, "malloc"); > - memset(blkstore, 0, blksize); > + err(1, "calloc"); > if (read(fd, blkstore, sb.st_size) != (ssize_t)sb.st_size) > err(1, "read"); > close(fd); -- "Action without study is fatal. Study without action is futile." -- Mary Ritter Beard
Re: [PATCH 9] installboot: malloc/memset => calloc
bump. anybody? On Sat, 31 May 2014 20:29:42 +0200 Benjamin Baier wrote: > On Sun, 1 Jun 2014 00:57:43 +1000 > Joel Sing wrote: > > In this case I think readability wins. I do not believe that there is a lot > > to > > gain from overflow protection given the numbers used in these calculations > > are very small (and many are already bounds checked in some form or other). > > Well, I had to ask. Here is option 2. Index: bootstrap.c === RCS file: /cvs/src/usr.sbin/installboot/bootstrap.c,v retrieving revision 1.3 diff -u -p -r1.3 bootstrap.c --- bootstrap.c 28 Dec 2013 12:01:33 - 1.3 +++ bootstrap.c 31 May 2014 18:14:56 - @@ -68,10 +68,9 @@ bootstrap(int devfd, char *dev, char *bo fprintf(stderr, "bootstrap is %zu bytes " "(%zu sectors @ %u bytes = %zu bytes)\n", (ssize_t)sb.st_size, bootsec, dl.d_secsize, bootsize); - boot = malloc(bootsize); + boot = calloc(1, bootsize); if (boot == NULL) - err(1, "malloc"); - memset(boot, 0, bootsize); + err(1, "calloc"); if (read(fd, boot, bootsize) != (ssize_t)sb.st_size) err(1, "read"); close(fd); Index: i386_softraid.c === RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v retrieving revision 1.1 diff -u -p -r1.1 i386_softraid.c --- i386_softraid.c 19 Jan 2014 02:58:50 - 1.1 +++ i386_softraid.c 31 May 2014 18:14:56 - @@ -129,11 +129,10 @@ sr_install_bootldr(int devfd, char *dev) inodeblk = nblocks - 1; bootsize = nblocks * SR_FS_BLOCKSIZE; - p = malloc(bootsize); + p = calloc(1, bootsize); if (p == NULL) err(1, NULL); - memset(p, 0, bootsize); fd = open(stage2, O_RDONLY, 0); if (fd == -1) err(1, NULL); Index: sparc64_installboot.c === RCS file: /cvs/src/usr.sbin/installboot/sparc64_installboot.c,v retrieving revision 1.1 diff -u -p -r1.1 sparc64_installboot.c --- sparc64_installboot.c 19 Jan 2014 02:58:50 - 1.1 +++ sparc64_installboot.c 31 May 2014 18:14:56 - @@ -68,10 +68,9 @@ md_loadboot(void) if (blksize > SBSIZE - DEV_BSIZE) errx(1, "boot blocks too big (%zu > %d)", blksize, SBSIZE - DEV_BSIZE); - blkstore = malloc(blksize); + blkstore = calloc(1, blksize); if (blkstore == NULL) - err(1, "malloc"); - memset(blkstore, 0, blksize); + err(1, "calloc"); if (read(fd, blkstore, sb.st_size) != (ssize_t)sb.st_size) err(1, "read"); close(fd);
Re: [PATCH 9] installboot: malloc/memset => calloc
On Sun, 1 Jun 2014 00:57:43 +1000 Joel Sing wrote: > In this case I think readability wins. I do not believe that there is a lot > to > gain from overflow protection given the numbers used in these calculations > are very small (and many are already bounds checked in some form or other). Well, I had to ask. Here is option 2. Index: bootstrap.c === RCS file: /cvs/src/usr.sbin/installboot/bootstrap.c,v retrieving revision 1.3 diff -u -p -r1.3 bootstrap.c --- bootstrap.c 28 Dec 2013 12:01:33 - 1.3 +++ bootstrap.c 31 May 2014 18:14:56 - @@ -68,10 +68,9 @@ bootstrap(int devfd, char *dev, char *bo fprintf(stderr, "bootstrap is %zu bytes " "(%zu sectors @ %u bytes = %zu bytes)\n", (ssize_t)sb.st_size, bootsec, dl.d_secsize, bootsize); - boot = malloc(bootsize); + boot = calloc(1, bootsize); if (boot == NULL) - err(1, "malloc"); - memset(boot, 0, bootsize); + err(1, "calloc"); if (read(fd, boot, bootsize) != (ssize_t)sb.st_size) err(1, "read"); close(fd); Index: i386_softraid.c === RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v retrieving revision 1.1 diff -u -p -r1.1 i386_softraid.c --- i386_softraid.c 19 Jan 2014 02:58:50 - 1.1 +++ i386_softraid.c 31 May 2014 18:14:56 - @@ -129,11 +129,10 @@ sr_install_bootldr(int devfd, char *dev) inodeblk = nblocks - 1; bootsize = nblocks * SR_FS_BLOCKSIZE; - p = malloc(bootsize); + p = calloc(1, bootsize); if (p == NULL) err(1, NULL); - memset(p, 0, bootsize); fd = open(stage2, O_RDONLY, 0); if (fd == -1) err(1, NULL); Index: sparc64_installboot.c === RCS file: /cvs/src/usr.sbin/installboot/sparc64_installboot.c,v retrieving revision 1.1 diff -u -p -r1.1 sparc64_installboot.c --- sparc64_installboot.c 19 Jan 2014 02:58:50 - 1.1 +++ sparc64_installboot.c 31 May 2014 18:14:56 - @@ -68,10 +68,9 @@ md_loadboot(void) if (blksize > SBSIZE - DEV_BSIZE) errx(1, "boot blocks too big (%zu > %d)", blksize, SBSIZE - DEV_BSIZE); - blkstore = malloc(blksize); + blkstore = calloc(1, blksize); if (blkstore == NULL) - err(1, "malloc"); - memset(blkstore, 0, blksize); + err(1, "calloc"); if (read(fd, blkstore, sb.st_size) != (ssize_t)sb.st_size) err(1, "read"); close(fd);
Re: [PATCH 9] installboot: malloc/memset => calloc
On Sat, 31 May 2014, Benjamin Baier wrote: > This one splits up the malloc parameter, taking full potential from calloc, > hurting readability a bit. which one is preferred? more > readable/maintainable or using the calloc overflow protection? In this case I think readability wins. I do not believe that there is a lot to gain from overflow protection given the numbers used in these calculations are very small (and many are already bounds checked in some form or other). > Index: bootstrap.c > === > RCS file: /cvs/src/usr.sbin/installboot/bootstrap.c,v > retrieving revision 1.3 > diff -u -p -r1.3 bootstrap.c > --- bootstrap.c 28 Dec 2013 12:01:33 - 1.3 > +++ bootstrap.c 31 May 2014 10:26:27 - > @@ -68,10 +68,9 @@ bootstrap(int devfd, char *dev, char *bo > fprintf(stderr, "bootstrap is %zu bytes " > "(%zu sectors @ %u bytes = %zu bytes)\n", > (ssize_t)sb.st_size, bootsec, dl.d_secsize, bootsize); > - boot = malloc(bootsize); > + boot = calloc(bootsec, dl.d_secsize); > if (boot == NULL) > - err(1, "malloc"); > - memset(boot, 0, bootsize); > + err(1, "calloc"); > if (read(fd, boot, bootsize) != (ssize_t)sb.st_size) > err(1, "read"); > close(fd); > Index: i386_softraid.c > === > RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v > retrieving revision 1.1 > diff -u -p -r1.1 i386_softraid.c > --- i386_softraid.c 19 Jan 2014 02:58:50 - 1.1 > +++ i386_softraid.c 31 May 2014 10:26:27 - > @@ -129,11 +129,10 @@ sr_install_bootldr(int devfd, char *dev) > inodeblk = nblocks - 1; > bootsize = nblocks * SR_FS_BLOCKSIZE; > > - p = malloc(bootsize); > + p = calloc(nblocks, SR_FS_BLOCKSIZE); > if (p == NULL) > err(1, NULL); > > - memset(p, 0, bootsize); > fd = open(stage2, O_RDONLY, 0); > if (fd == -1) > err(1, NULL); > Index: sparc64_installboot.c > === > RCS file: /cvs/src/usr.sbin/installboot/sparc64_installboot.c,v > retrieving revision 1.1 > diff -u -p -r1.1 sparc64_installboot.c > --- sparc64_installboot.c 19 Jan 2014 02:58:50 - 1.1 > +++ sparc64_installboot.c 31 May 2014 10:26:27 - > @@ -68,10 +68,9 @@ md_loadboot(void) > if (blksize > SBSIZE - DEV_BSIZE) > errx(1, "boot blocks too big (%zu > %d)", > blksize, SBSIZE - DEV_BSIZE); > - blkstore = malloc(blksize); > + blkstore = calloc(blocks, DEV_BSIZE); > if (blkstore == NULL) > - err(1, "malloc"); > - memset(blkstore, 0, blksize); > + err(1, "calloc"); > if (read(fd, blkstore, sb.st_size) != (ssize_t)sb.st_size) > err(1, "read"); > close(fd); -- "Action without study is fatal. Study without action is futile." -- Mary Ritter Beard
[PATCH 9] installboot: malloc/memset => calloc
This one splits up the malloc parameter, taking full potential from calloc, hurting readability a bit. which one is preferred? more readable/maintainable or using the calloc overflow protection? Index: bootstrap.c === RCS file: /cvs/src/usr.sbin/installboot/bootstrap.c,v retrieving revision 1.3 diff -u -p -r1.3 bootstrap.c --- bootstrap.c 28 Dec 2013 12:01:33 - 1.3 +++ bootstrap.c 31 May 2014 10:26:27 - @@ -68,10 +68,9 @@ bootstrap(int devfd, char *dev, char *bo fprintf(stderr, "bootstrap is %zu bytes " "(%zu sectors @ %u bytes = %zu bytes)\n", (ssize_t)sb.st_size, bootsec, dl.d_secsize, bootsize); - boot = malloc(bootsize); + boot = calloc(bootsec, dl.d_secsize); if (boot == NULL) - err(1, "malloc"); - memset(boot, 0, bootsize); + err(1, "calloc"); if (read(fd, boot, bootsize) != (ssize_t)sb.st_size) err(1, "read"); close(fd); Index: i386_softraid.c === RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v retrieving revision 1.1 diff -u -p -r1.1 i386_softraid.c --- i386_softraid.c 19 Jan 2014 02:58:50 - 1.1 +++ i386_softraid.c 31 May 2014 10:26:27 - @@ -129,11 +129,10 @@ sr_install_bootldr(int devfd, char *dev) inodeblk = nblocks - 1; bootsize = nblocks * SR_FS_BLOCKSIZE; - p = malloc(bootsize); + p = calloc(nblocks, SR_FS_BLOCKSIZE); if (p == NULL) err(1, NULL); - memset(p, 0, bootsize); fd = open(stage2, O_RDONLY, 0); if (fd == -1) err(1, NULL); Index: sparc64_installboot.c === RCS file: /cvs/src/usr.sbin/installboot/sparc64_installboot.c,v retrieving revision 1.1 diff -u -p -r1.1 sparc64_installboot.c --- sparc64_installboot.c 19 Jan 2014 02:58:50 - 1.1 +++ sparc64_installboot.c 31 May 2014 10:26:27 - @@ -68,10 +68,9 @@ md_loadboot(void) if (blksize > SBSIZE - DEV_BSIZE) errx(1, "boot blocks too big (%zu > %d)", blksize, SBSIZE - DEV_BSIZE); - blkstore = malloc(blksize); + blkstore = calloc(blocks, DEV_BSIZE); if (blkstore == NULL) - err(1, "malloc"); - memset(blkstore, 0, blksize); + err(1, "calloc"); if (read(fd, blkstore, sb.st_size) != (ssize_t)sb.st_size) err(1, "read"); close(fd);
