Date: Fri, 19 Feb 2016 10:47:13 -0800 From: John Baldwin <jhb@freebsd.org> To: Stefan Esser <se@freebsd.org> Cc: Stefan Esser <se@localhost.freebsd.org>, 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: <2165203.hVWJOExbtP@ralph.baldwin.cx> In-Reply-To: <56C6265F.4050101@freebsd.org> References: <201602181523.u1IFNQk8004338@repo.freebsd.org> <1865104.t06og7Ezta@ralph.baldwin.cx> <56C6265F.4050101@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday, February 18, 2016 09:15:27 PM Stefan Esser wrote:
> Am 18.02.2016 um 19:07 schrieb John Baldwin:
> >> @@ -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.
No, I think your current version is fine, was just lamenting the compilers. :)
> >> 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.
Thanks, the indentation is just a minor nit.
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2165203.hVWJOExbtP>