linux-kernel - Re: [PATCH 2/4] drivers core: Remove strcat uses around sysfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 08 Sep 2020 01:40:19 -0700
From:   Joe Perches <joe@...ches.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Pavel Machek <pavel@....cz>, Len Brown <len.brown@...el.com>,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH 2/4] drivers core: Remove strcat uses around sysfs_emit
 and neaten

On Tue, 2020-09-08 at 10:32 +0200, Greg Kroah-Hartman wrote:
> On Mon, Sep 07, 2020 at 10:58:06AM -0700, Joe Perches wrote:
> > strcat is no longer necessary for sysfs_emit and sysfs_emit_at uses.
> > 
> > Convert the strcat uses to sysfs_emit calls and neaten other block
> > uses of direct returns to use an intermediate const char *.
[]
> This function is now longer, with an assignment that is not needed
> (type=NULL), so why make this "cleanup"?

It's smaller object code.

[]
> > Again, not a type, it's a state.  And you did not merge all sysfs_emit()
> calls into one here, so it's messier now, don't you think?

You can't because the default type uses an argument and not
a fixed string.  I don't think it's messier, no.

> >  int memory_notify(unsigned long val, void *v)
> > @@ -307,17 +305,16 @@ static ssize_t phys_device_show(struct device *dev,
> >  }
> >  
> >  #ifdef CONFIG_MEMORY_HOTREMOVE
> > -static void print_allowed_zone(char *buf, int nid, unsigned long start_pfn,
> > -		unsigned long nr_pages, int online_type,
> > -		struct zone *default_zone)
> > +static int print_allowed_zone(char *buf, int len, int nid,
> > +			      unsigned long start_pfn, unsigned long nr_pages,
> > +			      int online_type, struct zone *default_zone)
> 
> Unrelated change :(

No it's not, it's outputting to buf so it
needs len to output to appropriate spot to
be able to use sysfs_emit_at.

> >  {
> >  	struct zone *zone;
> >  
> >  	zone = zone_for_pfn_range(online_type, nid, start_pfn, nr_pages);
> > -	if (zone != default_zone) {
> > -		strcat(buf, " ");
> > -		strcat(buf, zone->name);
> > -	}
> > +	if (zone == default_zone)
> > +		return 0;
> > +	return sysfs_emit_at(buf, len, " %s", zone->name);

here.

> []

> This is better.

All of it is better. <smile>

> > diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
[]
> > @@ -255,9 +255,9 @@ static ssize_t pm_qos_latency_tolerance_us_show(struct device *dev,
> >  	s32 value = dev_pm_qos_get_user_latency_tolerance(dev);
> >  
> >  	if (value < 0)
> > -		return sysfs_emit(buf, "auto\n");
> > +		return sysfs_emit(buf, "%s\n", "auto");
> >  	if (value == PM_QOS_LATENCY_ANY)
> > -		return sysfs_emit(buf, "any\n");
> > +		return sysfs_emit(buf, "%s\n", "any");
> >  
> >  	return sysfs_emit(buf, "%d\n", value);
> >  }
> 
> Unrelated change :(

Again, no it's not unrelated, it's consistent.