On Mon, 2008-11-17 at 21:59 +0100, Martin Mares wrote:
> > static void
> > +cap_vpd(struct device *d)
> > +{
> > + word res_addr = 0, res_len, part_pos, part_len;
> > + byte key[2], buf[256];
> > + byte tag;
> > +
> > + printf("Vital Product Data\n");
> > +
> > + while (res_addr <= PCI_VPD_ADDR_MASK) {
> > + if (!pci_read_vpd(d->dev, res_addr, &tag, 1))
> > + break;
>
> Please keep the indentation style consistent with the rest of the file.
Sorry, I didn't spot the double-indentation inside braces.
> > + case 0x82:
> > + printf("\t\tProduct Name: ");
> > + while (part_pos < res_len) {
> > + part_len = res_len - part_pos;
> > + if (part_len > sizeof(buf))
> > + part_len = sizeof(buf);
> > + if (!pci_read_vpd(d->dev, res_addr + part_pos, buf, part_len))
> > + break;
> > + fwrite(buf, 1, part_len, stdout);
>
> You should definitely check whether the characters are printable
> (both here and in the Alphanumeric content case).
How about:
static void print_vpd_string(const u8 *buf, size_t len)
{
while (len--)
{
unsigned char ch = *buf++;
if (ch == '\\')
printf("\\\\");
else if (ch < 32 || ch == 127)
printf("\\x%02x", ch);
else
putchar(ch);
}
}
> > + if ((key[0] == 'E' && key[1] == 'C') ||
> > + (key[0] == 'P' && key[1] == 'N') ||
> > + (key[0] == 'S' && key[1] == 'N') ||
> > + key[0] == 'V' ||
> > + key[0] == 'Y') {
> > + /* Alphanumeric content */
> > + printf("\t\t\t%c%c: %.*s\n", key[0], key[1], part_len, buf);
>
> I think that it would be nice to accompany the attributes which we
> recognize with an explanation (a more verbose name).
[...]
Could do, but it's not critical.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
