Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1444123

Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance Monitoring Unit driver

Show all headers | View raw

On Thu, Jul 14, 2016 at 11:05:48AM -0700, Tai Tri Nguyen wrote:
> Hi Joe,
> 
> On Thu, Jul 14, 2016 at 10:54 AM, Tai Tri Nguyen <ttnguyen@apm.com> wrote:
> > Hi Joe,
> >
> > On Thu, Jul 14, 2016 at 10:47 AM, Joe Perches <joe@perches.com> wrote:
> >> On Thu, 2016-07-14 at 10:27 -0700, Tai Nguyen wrote:
> >>> This patch adds a driver for the SoC-wide (AKA uncore) PMU hardware
> >>> found in APM X-Gene SoCs.
> >>
> >> trivia:
> >>
> >>> diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
> >> []
> >>> +struct xgene_pmu_dev_ctx {
> >>> +     char *name;
> >>> +     struct list_head next;
> >>> +     struct xgene_pmu_dev *pmu_dev;
> >>> +     struct hw_pmu_info inf;
> >>> +};
> >>
> >> Probably better to use something like
> >>         char    name[20];
> >> as the kasprintf can fail and this doesn't
> >> seem to be freed anywhere.
> >
> > Okay. I'll fix it shortly.
> >
> 
> I take it back.
> I refer many other drivers using kasprintf and they do the same way I do.
> Can you please check it again?

Joe is correct that you allocate a string with kasprintf, and this never
gets freed, even if the driver is removed. Thus, memory may be leaked.

If other drivers do the same, they are similarly wrong.

Even if this is a rare case, it's not good practice to leave allocations
unbalanced. So please fix this.

If you don't want to change the struct, another option is to use
devm_kasprintf. However, I suspect with all the accounting data
structures that will take up more space.

Thanks,
Mark.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v9 0/4] perf: Add APM X-Gene SoC Performance Monitoring Unit driver  Tai Nguyen <ttnguyen@apm.com> - 2016-07-14 19:40 +0200
  [PATCH v9 2/4] Documentation: Add documentation for APM X-Gene SoC PMU DTS binding Tai Nguyen <ttnguyen@apm.com> - 2016-07-14 19:40 +0200
  [PATCH v9 1/4] MAINTAINERS: Add entry for APM X-Gene SoC PMU driver Tai Nguyen <ttnguyen@apm.com> - 2016-07-14 19:40 +0200
  [PATCH v9 4/4] arm64: dts: apm: Add APM X-Gene SoC PMU DTS entries Tai Nguyen <ttnguyen@apm.com> - 2016-07-14 19:40 +0200
  Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance  Monitoring Unit driver Joe Perches <joe@perches.com> - 2016-07-14 19:50 +0200
    Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance  Monitoring Unit driver Tai Tri Nguyen <ttnguyen@apm.com> - 2016-07-14 20:00 +0200
      Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance  Monitoring Unit driver Tai Tri Nguyen <ttnguyen@apm.com> - 2016-07-14 20:10 +0200
        Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance  Monitoring Unit driver Mark Rutland <mark.rutland@arm.com> - 2016-07-15 11:50 +0200
          Re: [PATCH v9 3/4] perf: xgene: Add APM X-Gene SoC Performance  Monitoring Unit driver Tai Tri Nguyen <ttnguyen@apm.com> - 2016-07-15 19:40 +0200

csiph-web