Date: Thu, 18 Feb 2016 21:15:27 +0100 From: Stefan Esser <se@freebsd.org> To: John Baldwin <jhb@freebsd.org>, Stefan Esser <se@localhost.FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r295760 - head/usr.sbin/pciconf Message-ID: <56C6265F.4050101@freebsd.org> In-Reply-To: <1865104.t06og7Ezta@ralph.baldwin.cx> References: <201602181523.u1IFNQk8004338@repo.freebsd.org> <1865104.t06og7Ezta@ralph.baldwin.cx>
next in thread | previous in thread | raw e-mail | index | archive | help
Am 18.02.2016 um 19:07 schrieb John Baldwin: > On Thursday, February 18, 2016 03:23:26 PM Stefan Esser wrote: >> Author: se >> Date: Thu Feb 18 15:23:25 2016 >> New Revision: 295760 >> URL: https://svnweb.freebsd.org/changeset/base/295760 >> >> Log: >> Make WARNS=6 safe. >> >> Tested with Clang 3.7.1, GCC 4.2.1 and GCC 4.8.5 on amd64. > > Thanks. > >> Modified: head/usr.sbin/pciconf/cap.c >> ============================================================================== >> --- head/usr.sbin/pciconf/cap.c Thu Feb 18 15:12:52 2016 (r295759) >> +++ head/usr.sbin/pciconf/cap.c Thu Feb 18 15:23:25 2016 (r295760) >> @@ -120,6 +120,9 @@ static void >> cap_vpd(int fd, struct pci_conf *p, uint8_t ptr) >> { >> >> + (void)fd; /* UNUSED */ >> + (void)p; /* UNUSED */ >> + (void)ptr; /* UNUSED */ >> printf("VPD"); >> } > > I think we prefer __unused in the parameter declaration instead? That is: Since I was not sure about the best way to silence this warning, I had looked at the mail archive and found a mail thread from July 2012 with subject "(void)foo or __unused foo ?". The cast to void seemed to be the most portable method, but in fact I prefer the __unused modifier myself. > cap_vpd(int fd __unused, struct pci_conf *p __unused, uint8_t ptr __unused) Yes, I'll commit that version in a minute. >> @@ -172,6 +175,7 @@ cap_pcix(int fd, struct pci_conf *p, uin >> } >> if ((p->pc_hdr & PCIM_HDRTYPE) == 1) >> return; >> + max_burst_read = 0; >> switch (status & PCIXM_STATUS_MAX_READ) { >> case PCIXM_STATUS_MAX_READ_512: >> max_burst_read = 512; > > Compilers are simply not smart enough. :-P Well, clang-3.7.1 in -CURRENT is ... But both gcc-4.2.1 and gcc-4.8.5 warn about a possibly uninitialized variable. I could have changed the last case selector into "default" to silence the warning without need for an initializer before the switch statement, but that would be too ugly and misleading. >> Modified: head/usr.sbin/pciconf/pciconf.c >> ============================================================================== >> --- head/usr.sbin/pciconf/pciconf.c Thu Feb 18 15:12:52 2016 (r295759) >> +++ head/usr.sbin/pciconf/pciconf.c Thu Feb 18 15:23:25 2016 (r295760) >> @@ -913,7 +915,8 @@ parsesel(const char *str) >> ep += 3; >> i = 0; >> do { >> - selarr[i++] = strtoul(ep, &ep, 10); >> + selarr[i++] = strtoul(ep, &eppos, 10); >> + ep = eppos; >> } while ((*ep == ':' || *ep == '.') && *++ep != '\0' && i < 4); > > This is now indented oddly (2 spaces instead of a tab?). Sorry, the pciconf sources do not comply with "style" and I'm to blame, since I committed the initial version nearly 20 years ago ... But you are correct, I got even the wrong indentation wrong ;-) Thank you for the review and your comments, I'll commit a fixed version now. Best regards, STefan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?56C6265F.4050101>