From patchwork Fri Nov 28 16:38:07 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leif Lindholm X-Patchwork-Id: 41709 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f72.google.com (mail-la0-f72.google.com [209.85.215.72]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 2F27D244C2 for ; Fri, 28 Nov 2014 16:38:17 +0000 (UTC) Received: by mail-la0-f72.google.com with SMTP id gq15sf4417391lab.7 for ; Fri, 28 Nov 2014 08:38:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:cc:subject:message-id :references:mime-version:content-type:content-disposition :in-reply-to:user-agent:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=Kyu8rIrcnL1e33MyxOjr2rapzjihKbevavOz4gRzd0s=; b=ax5A2iwQnCSDrhu2WYdvEdFjbONf9YpzFSzz3sGWK5db2wU0ODqNdIX2OsQcxzetu7 iE3jyo6y14AfXAvgqaduYXmiZA7420yBAqX+Ji4DODzApAJf3XiUyvB5MBAatY1eSk17 XNrH09G/h7gqusVp461pMjobNKKtW38uemjlwytApBDVzKLurn1UcucJi60kllD5Yslg tk/Ks9iuiXzR3QCaDihfj/Utlywbx83oiOX378Yh39jNV9xcgVMF9bf1OHVfNXu3/ZmT 5BTuvxiQKmn7LVE/0SzrDEoq2A1ocnhIE5S8DgBRpQaGntD34Slvg5APCMi3w/ueOAKS 0n6g== X-Gm-Message-State: ALoCoQn3cPPdxu6qtFYKf0cutySJo4u+uN7UND11maa5sYgh5RJXjLr8Afec6RUlxBgqVNUliIPK X-Received: by 10.112.151.38 with SMTP id un6mr12310869lbb.0.1417192696106; Fri, 28 Nov 2014 08:38:16 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.5.202 with SMTP id u10ls749786lau.68.gmail; Fri, 28 Nov 2014 08:38:15 -0800 (PST) X-Received: by 10.112.132.2 with SMTP id oq2mr44828540lbb.11.1417192695953; Fri, 28 Nov 2014 08:38:15 -0800 (PST) Received: from mail-lb0-f175.google.com (mail-lb0-f175.google.com. [209.85.217.175]) by mx.google.com with ESMTPS id kl4si2385662lbc.134.2014.11.28.08.38.15 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 28 Nov 2014 08:38:15 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.175 as permitted sender) client-ip=209.85.217.175; Received: by mail-lb0-f175.google.com with SMTP id u10so5731654lbd.6 for ; Fri, 28 Nov 2014 08:38:15 -0800 (PST) X-Received: by 10.112.189.10 with SMTP id ge10mr44077751lbc.23.1417192695829; Fri, 28 Nov 2014 08:38:15 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.184.201 with SMTP id ew9csp120376lbc; Fri, 28 Nov 2014 08:38:14 -0800 (PST) X-Received: by 10.66.228.131 with SMTP id si3mr75913254pac.32.1417192694111; Fri, 28 Nov 2014 08:38:14 -0800 (PST) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id gz1si16989665pbc.65.2014.11.28.08.38.13 for ; Fri, 28 Nov 2014 08:38:14 -0800 (PST) Received-SPF: none (google.com: devicetree-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751493AbaK1QiM (ORCPT + 4 others); Fri, 28 Nov 2014 11:38:12 -0500 Received: from mail-wi0-f182.google.com ([209.85.212.182]:50394 "EHLO mail-wi0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751111AbaK1QiM (ORCPT ); Fri, 28 Nov 2014 11:38:12 -0500 Received: by mail-wi0-f182.google.com with SMTP id h11so11714015wiw.9 for ; Fri, 28 Nov 2014 08:38:11 -0800 (PST) X-Received: by 10.180.78.225 with SMTP id e1mr41788608wix.32.1417192690882; Fri, 28 Nov 2014 08:38:10 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by mx.google.com with ESMTPSA id pu3sm15670451wjc.14.2014.11.28.08.38.09 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 28 Nov 2014 08:38:10 -0800 (PST) Date: Fri, 28 Nov 2014 16:38:07 +0000 From: Leif Lindholm To: Grant Likely Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, mark.rutland@arm.com, robh+dt@kernel.org, plagnioj@jcrosoft.com, ijc@debian.org, andrew@lunn.ch, s.hauer@pengutronix.de Subject: Re: [PATCH v3 2/3] of: add optional options parameter to? of_find_node_by_path() Message-ID: <20141128163807.GN2361@bivouac.eciton.net> References: <1417110967-16284-1-git-send-email-leif.lindholm@linaro.org> <1417110967-16284-3-git-send-email-leif.lindholm@linaro.org> <20141128004403.08D90C40A7F@trevor.secretlab.ca> <20141128113428.GM2361@bivouac.eciton.net> <20141128152512.2152EC40884@trevor.secretlab.ca> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20141128152512.2152EC40884@trevor.secretlab.ca> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: devicetree-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: devicetree@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: leif.lindholm@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.175 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , On Fri, Nov 28, 2014 at 03:25:12PM +0000, Grant Likely wrote: > On Fri, 28 Nov 2014 11:34:28 +0000 > , Leif Lindholm > wrote: > > On Fri, Nov 28, 2014 at 12:44:03AM +0000, Grant Likely wrote: > > > > + separator = strchr(path, ':'); > > > > + if (separator && opts) > > > > + *opts = separator + 1; > > > > + > > > > > > What about when there are no opts? Do we require the caller to make sure > > > opts is NULL before calling the function (which sounds like a good > > > source of bugs) or do we clear it on successful return? > > > > > > I think if opts is passed in, but there are no options, then it should > > > set *opts = NULL. > > > > Yeah, oops. > > > > > There should be test cases for this also. Must set opts to NULL on > > > successful return, and (I think) should leave opts alone on an > > > unsuccessful search. > > > > I would actually argue for always nuking the opts - since that could > > (theoretically) prevent something working by accident in spite of > > error conditions. > > > > How about the below? > > Perfect, applied with one fixup below... Thanks! And since it's Friday... *cough* >From 5c469dad81961164c444da7d6c4e1c5b1c097aab Mon Sep 17 00:00:00 2001 From: Leif Lindholm Date: Fri, 28 Nov 2014 16:27:25 +0000 Subject: [PATCH] of: fix options clearing when path is "/" The addition of the optional options extraction on of_find_node_opts_by_path failed to clear incoming options pointer if the specified path was "/". Resolve this case, and add a test to detect it. Signed-off-by: Leif Lindholm --- drivers/of/base.c | 11 ++++++----- drivers/of/selftest.c | 5 +++++ 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 5e16c6b..32664d1 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -743,15 +743,16 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt struct device_node *np = NULL; struct property *pp; unsigned long flags; - char *separator; + char *separator = NULL; + + if (opts) { + separator = strchr(path, ':'); + *opts = separator ? separator + 1 : NULL; + } if (strcmp(path, "/") == 0) return of_node_get(of_allnodes); - separator = strchr(path, ':'); - if (opts) - *opts = separator ? separator + 1 : NULL; - /* The path could begin with an alias */ if (*path != '/') { char *p = strchrnul(path, '/'); diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c index 721b2a4..914f0ae 100644 --- a/drivers/of/selftest.c +++ b/drivers/of/selftest.c @@ -100,6 +100,11 @@ static void __init of_selftest_find_node_by_name(void) np = of_find_node_opts_by_path("testcase-alias", &options); selftest(np && !options, "option clearing test failed\n"); of_node_put(np); + + options = "testoption"; + np = of_find_node_opts_by_path("/", &options); + selftest(np && !options, "option clearing root node test failed\n"); + of_node_put(np); } static void __init of_selftest_dynamic(void)