From patchwork Wed Sep 8 20:24:20 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Miller X-Patchwork-Id: 64203 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 2D7E1B6EF2 for ; Thu, 9 Sep 2010 06:24:10 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754632Ab0IHUYF (ORCPT ); Wed, 8 Sep 2010 16:24:05 -0400 Received: from 74-93-104-97-Washington.hfc.comcastbusiness.net ([74.93.104.97]:34311 "EHLO sunset.davemloft.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754423Ab0IHUYD (ORCPT ); Wed, 8 Sep 2010 16:24:03 -0400 Received: from localhost (localhost [127.0.0.1]) by sunset.davemloft.net (Postfix) with ESMTP id EBDCF24C08A; Wed, 8 Sep 2010 13:24:20 -0700 (PDT) Date: Wed, 08 Sep 2010 13:24:20 -0700 (PDT) Message-Id: <20100908.132420.179918700.davem@davemloft.net> To: error27@gmail.com Cc: vladislav.yasevich@hp.com, sri@us.ibm.com, yjwei@cn.fujitsu.com, cascardo@holoscopio.com, linux-sctp@vger.kernel.org, netdev@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [patch] sctp: fix test for end of loop From: David Miller In-Reply-To: <20100906122344.GA2764@bicker> References: <20100906122344.GA2764@bicker> X-Mailer: Mew version 6.3 on Emacs 23.1 / Mule 6.0 (HANACHIRUSATO) Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Dan Carpenter Date: Mon, 6 Sep 2010 14:26:39 +0200 > "new_addr" is the list cursor here and it's always non-NULL. > > We're trying to test if we exited because the loop ended or we hit the > break statement. Really testing !found is enough so long as > "new_asoc->peer.transport_addr_list" is not empty and I believe it never > is empty at this point. So this is never really a bug with the current > code. > > Signed-off-by: Dan Carpenter If you don't mind, I still think the code is confusing after your patch even if the result is correct. What do you think about the following kind of approach instead? Thanks! --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 8b28443..3d5bbae7 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -1241,7 +1241,7 @@ static int sctp_sf_check_restart_addrs(const struct sctp_association *new_asoc, sctp_cmd_seq_t *commands) { struct sctp_transport *new_addr, *addr; - int found; + int ret = 1; /* Implementor's Guide - Sectin 5.2.2 * ... @@ -1254,31 +1254,28 @@ static int sctp_sf_check_restart_addrs(const struct sctp_association *new_asoc, /* Search through all current addresses and make sure * we aren't adding any new ones. */ - new_addr = NULL; - found = 0; - list_for_each_entry(new_addr, &new_asoc->peer.transport_addr_list, transports) { - found = 0; list_for_each_entry(addr, &asoc->peer.transport_addr_list, transports) { if (sctp_cmp_addr_exact(&new_addr->ipaddr, - &addr->ipaddr)) { - found = 1; - break; - } + &addr->ipaddr)) + goto next; } - if (!found) - break; - } - /* If a new address was added, ABORT the sender. */ - if (!found && new_addr) { + /* 'new_addr' could not be found in the transport address + * list of 'asoc', abort. + */ sctp_sf_send_restart_abort(&new_addr->ipaddr, init, commands); + ret = 0; + break; + + next: + ; } /* Return success if all addresses were found. */ - return found; + return ret; } /* Populate the verification/tie tags based on overlapping INIT