From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson Subject: Re: [PATCH 4/4] Create a new property value that means 'undefined'. Date: Mon, 25 Oct 2010 11:44:18 +1100 Message-ID: <20101025004418.GC5355@yookeroo> References: <20101020214419.2985.51068.stgit@riker> <20101020214522.2985.97224.stgit@riker> <20101021061409.GJ6227@yookeroo> <1287777058.4535.1571.camel@riker> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1287777058.4535.1571.camel@riker> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org To: John Bonesio Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org List-Id: devicetree@vger.kernel.org On Fri, Oct 22, 2010 at 12:50:58PM -0700, John Bonesio wrote: > On Thu, 2010-10-21 at 17:14 +1100, David Gibson wrote: > > On Wed, Oct 20, 2010 at 02:45:22PM -0700, John Bonesio wrote: [snip] > > > @@ -188,6 +197,25 @@ void add_property(struct node *node, struct property *prop) > > > *p = prop; > > > } > > > > > > +void remove_property(struct node *node, struct property *prop) > > > +{ > > > + struct property **p; > > > + int found = 0; > > > + > > > + p = &node->proplist; > > > + while (*p) { > > > + if (*p == prop) { > > > + *p = (*p)->next; > > > + found = 1; > > > + break; > > > + } > > > + p = &((*p)->next); > > > + } > > > + /* property not in the node? it's probably an error, so flag it. */ > > > + assert(found); > > > > assert()s are for finding bugs in code, not detecting errors in user > > input. AFAICT this *can* be generated by usre input so it should > > print an error message and if possible continue, not cause a SIGABRT. > > I agree that assert() shouldn't be used if it can be caused by user > input, but I don't see how the assert can be triggered by user input. In > my mind this is a programming error. > > I the only way I see this being triggered by user input is if the user > specifies that a property be deleted but it doesn't exist in the current > in-memory merged tree. I believe the merge_nodes() routine takes care > to avoid problems here by making sure the property already exists in the > in-memory merged tree before calling remove_property() on it. Well, it does, but remove_property() is called *from* merge_nodes() and it's called *before* the new property is added to the old node. So, unless I'm missing something even more subtle, the user attempting to remove a property which does not exist will indeed trigger this assert(). This would make a good testcase... -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson