Messages in this thread |  | | | Date | Fri, 13 Feb 2009 01:04:02 -0500 (EST) | | From | Steven Rostedt <> | | Subject | Re: [PATCH 2/2] tracing: add checks for printing pci devices in mmiotrace. |
| |
On Thu, 12 Feb 2009, Wenji Huang wrote:
> This patch is to provide checking return value of trace_seq_printf > so as to keep completeness of mmio_print_pcidev. > > Signed-off-by: Wenji Huang <wenji.huang@oracle.com> > --- > kernel/trace/trace_mmiotrace.c | 26 ++++++++++++++++---------- > 1 files changed, 16 insertions(+), 10 deletions(-) > > diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c > index c401b90..ebb0b61 100644 > --- a/kernel/trace/trace_mmiotrace.c > +++ b/kernel/trace/trace_mmiotrace.c > @@ -56,17 +56,19 @@ static void mmio_trace_start(struct trace_array *tr) > mmio_reset_data(tr); > } > > -static int mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev) > +static enum print_line_t mmio_print_pcidev(struct trace_seq *s, > + const struct pci_dev *dev) > { > int ret = 0; > int i; > resource_size_t start, end; > const struct pci_driver *drv = pci_dev_driver(dev); > > - /* XXX: incomplete checks for trace_seq_printf() return value */ > - ret += trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x", > + ret = trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x", > dev->bus->number, dev->devfn, > dev->vendor, dev->device, dev->irq); > + if (!ret) > + return TRACE_TYPE_PARTIAL_LINE; > /* > * XXX: is pci_resource_to_user() appropriate, since we are > * supposed to interpret the __ioremap() phys_addr argument based on > @@ -74,21 +76,25 @@ static int mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev) > */ > for (i = 0; i < 7; i++) { > pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > - ret += trace_seq_printf(s, " %llx", > + ret = trace_seq_printf(s, " %llx", > (unsigned long long)(start | > (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); > - } > - for (i = 0; i < 7; i++) { > + if (!ret) > + return TRACE_TYPE_PARTIAL_LINE;
The above looks like a change of logic to me. I'm a bit tired so I can be wrong. But it looks like the original was:
for (i = 0; i < 7; i++) { pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); ret += trace_seq_printf(s, " %llx", (unsigned long long)(start | (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); } for (i = 0; i < 7; i++) { pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); ret += trace_seq_printf(s, " %llx", dev->resource[i].start < dev->resource[i].end ? (unsigned long long)(end - start) + 1 : 0); } And the new is:
for (i = 0; i < 7; i++) { pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); ret = trace_seq_printf(s, " %llx", (unsigned long long)(start | (dev->resource[i].flags & PCI_REGION_FLAG_MASK))); if (!ret) return TRACE_TYPE_PARTIAL_LINE; pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); ret = trace_seq_printf(s, " %llx", dev->resource[i].start < dev->resource[i].end ? (unsigned long long)(end - start) + 1 : 0); if (!ret) return TRACE_TYPE_PARTIAL_LINE; } This changes the output format.
Pekka,
Is this OK?
-- Steve
> pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > - ret += trace_seq_printf(s, " %llx", > + ret = trace_seq_printf(s, " %llx", > dev->resource[i].start < dev->resource[i].end ? > (unsigned long long)(end - start) + 1 : 0); > + if (!ret) > + return TRACE_TYPE_PARTIAL_LINE; > } > if (drv) > - ret += trace_seq_printf(s, " %s\n", drv->name); > + ret = trace_seq_printf(s, " %s\n", drv->name); > else > - ret += trace_seq_printf(s, " \n"); > - return ret; > + ret = trace_seq_printf(s, " \n"); > + if (!ret) > + return TRACE_TYPE_PARTIAL_LINE; > + return TRACE_TYPE_HANDLED; > } > > static void destroy_header_iter(struct header_iter *hiter) > -- > 1.5.6 > > >
|  |