From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941703AbcIHPqJ (ORCPT ); Thu, 8 Sep 2016 11:46:09 -0400 Received: from mx0b-001ae601.pphosted.com ([67.231.152.168]:39922 "EHLO mx0b-001ae601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932741AbcIHPqI (ORCPT ); Thu, 8 Sep 2016 11:46:08 -0400 Authentication-Results: ppops.net; spf=none smtp.mailfrom=rf@opensource.wolfsonmicro.com Message-ID: <1473349561.4359.23.camel@rf-debian.wolfsonmicro.main> Subject: Re: [PATCH 2/2] of: Add array read functions with min/max size limits From: Richard Fitzgerald To: Rob Herring CC: Frank Rowand , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Date: Thu, 8 Sep 2016 16:46:01 +0100 In-Reply-To: References: <1473174121-19742-1-git-send-email-rf@opensource.wolfsonmicro.com> <1473174121-19742-2-git-send-email-rf@opensource.wolfsonmicro.com> <1473348852.4359.18.camel@rf-debian.wolfsonmicro.main> Content-Type: text/plain; charset="us-ascii" X-Mailer: Evolution 3.4.4-3 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 impostorscore=0 lowpriorityscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1609080228 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2016-09-08 at 10:38 -0500, Rob Herring wrote: > On Thu, Sep 8, 2016 at 10:34 AM, Richard Fitzgerald > wrote: > > On Thu, 2016-09-08 at 09:46 -0500, Rob Herring wrote: > >> On Tue, Sep 6, 2016 at 10:02 AM, Richard Fitzgerald > >> wrote: > >> > Add a new set of array reading functions that take a minimum and > >> > maximum size limit and will fail if the property size is not within > >> > the size limits. This makes it more convenient for drivers that > >> > use variable-size DT arrays which must be bounded at both ends - > >> > data must be at least N entries but must not overflow the array > >> > it is being copied into. It is also more efficient than making this > >> > functionality out of existing public functions and avoids duplication. > >> > > >> > The existing array functions have been left in the API, since there > >> > are a very large number of clients of those functions and their > >> > existing functionality is still useful. This avoids turning a small > >> > API improvement into a major kernel rework. > > [...] > > >> > @@ -1229,21 +1270,53 @@ EXPORT_SYMBOL_GPL(of_property_read_u32_index); > >> > int of_property_read_u8_array(const struct device_node *np, > >> > const char *propname, u8 *out_values, size_t sz) > >> > { > >> > - const u8 *val = of_find_property_value_of_size(np, propname, > >> > - (sz * sizeof(*out_values)), > >> > - 0, > >> > - NULL); > >> > - > >> > - if (IS_ERR(val)) > >> > - return PTR_ERR(val); > >> > - > >> > - while (sz--) > >> > - *out_values++ = *val++; > >> > - return 0; > >> > + return of_property_read_variable_u8_array(np, propname, out_values, > >> > + sz, 0); > >> > >> This should be min and max both set to sz. > > > > Passing 0 as max preserves the existing behaviour of these functions of > > only requiring the array to be at least sz long, but not caring if it's > > longer. > > Yes, I was just writing to say that after reading patch 1 more carefully. > Although at the same time I was realizing that actually my code is broken for that and somehow I missed validating it in my testing. The new functions validate the min and max against the DT entry size and copy the actual number of elements. The old functions effectively validate only the min and return that number of elements. I'm just considering whether it's worth fixing my new functions to keep the intention that max=0 duplicates the old behaviour, or not bother and just keep the old functions. > Rob