public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed

From: Nathan Lynch <nathanl@linux•ibm.com>
To: Pingfan Liu <kernelfans@gmail•com>
Cc: Andrew Donnellan <ajd@linux•ibm.com>,
	"Aneesh Kumar K . V" <aneesh.kumar@linux•ibm.com>,
	kexec@lists•infradead.org, linuxppc-dev@lists•ozlabs.org,
	Pingfan Liu <kernelfans@gmail•com>,
	Rob Herring <robh+dt@kernel•org>,
	Paul Mackerras <paulus@samba•org>,
	Oliver O'Halloran <oohall@gmail•com>,
	Dan Williams <dan.j.williams@intel•com>,
	Frank Rowand <frowand.list@gmail•com>,
	Hari Bathini <hbathini@linux•ibm.com>
Subject: Re: [PATCHv3 1/2] powerpc/of: split out new_property() for reusing
Date: Fri, 06 Mar 2020 13:59:40 -0600	[thread overview]
Message-ID: <87eeu5jlub.fsf@linux.ibm.com> (raw)
In-Reply-To: <1583311651-29310-2-git-send-email-kernelfans@gmail.com>

Hi,

Pingfan Liu <kernelfans@gmail•com> writes:
> Splitting out new_property() for coming reusing and moving it to
> of_helpers.c.

[...]

> +struct property *new_property(const char *name, const int length,
> +		const unsigned char *value, struct property *last)
> +{
> +	struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
> +
> +	if (!new)
> +		return NULL;
> +
> +	new->name = kstrdup(name, GFP_KERNEL);
> +	if (!new->name)
> +		goto cleanup;
> +	new->value = kmalloc(length + 1, GFP_KERNEL);
> +	if (!new->value)
> +		goto cleanup;
> +
> +	memcpy(new->value, value, length);
> +	*(((char *)new->value) + length) = 0;
> +	new->length = length;
> +	new->next = last;
> +	return new;
> +
> +cleanup:
> +	kfree(new->name);
> +	kfree(new->value);
> +	kfree(new);
> +	return NULL;
> +}

This function in its current form isn't suitable for more general use:

* It appears to be tailored to string properties - note the char * value
  parameter, the length + 1 allocation and nul termination.

* Most code shouldn't need the 'last' argument. The code where this
  currently resides builds a list of properties and attaches it to a new
  node, bypassing of_add_property().

Let's look at the call site you add in your next patch:

+		big = cpu_to_be64(p->bound_addr);
+		property = new_property("bound-addr", sizeof(u64), (const unsigned char *)&big,
+			NULL);
+		of_add_property(dn, property);

So you have to use a cast, and this is going to allocate (sizeof(u64) + 1)
for the value, is that what you want?

I think you should leave that legacy pseries reconfig code undisturbed
(frankly that stuff should get deprecated and removed) and if you want a
generic helper it should look more like:

struct property *of_property_new(const char *name, size_t length,
                                 const void *value, gfp_t allocflags)

__of_prop_dup() looks like a good model/guide here.

next prev parent reply	other threads:[~2020-03-06 20:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-28  9:41 [PATCHv2 1/2] powerpc/of: split out new_property() for reusing Pingfan Liu
2020-02-28  9:41 ` [PATCHv2 2/2] pSeries/papr_scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu
2020-03-04  8:47 ` [PATCHv3 0/2] pseries/scm: " Pingfan Liu
2020-03-04  8:47   ` [PATCHv3 1/2] powerpc/of: split out new_property() for reusing Pingfan Liu
2020-03-05  3:58     ` Andrew Donnellan
2020-03-06 19:59     ` Nathan Lynch [this message]
2020-03-09  1:50       ` Pingfan Liu
2020-03-04  8:47   ` [PATCHv3 2/2] pseries/scm: buffer pmem's bound addr in dt for kexec kernel Pingfan Liu
2020-03-13  3:17     ` Oliver O'Halloran
2020-03-16  2:49       ` Pingfan Liu
2020-03-16  2:53     ` Aneesh Kumar K.V
2020-03-16  8:37       ` Pingfan Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87eeu5jlub.fsf@linux.ibm.com \
    --to=nathanl@linux$(echo .)ibm.com \
    --cc=ajd@linux$(echo .)ibm.com \
    --cc=aneesh.kumar@linux$(echo .)ibm.com \
    --cc=dan.j.williams@intel$(echo .)com \
    --cc=frowand.list@gmail$(echo .)com \
    --cc=hbathini@linux$(echo .)ibm.com \
    --cc=kernelfans@gmail$(echo .)com \
    --cc=kexec@lists$(echo .)infradead.org \
    --cc=linuxppc-dev@lists$(echo .)ozlabs.org \
    --cc=oohall@gmail$(echo .)com \
    --cc=paulus@samba$(echo .)org \
    --cc=robh+dt@kernel$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox