Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 9 Jun 2022 21:34:19 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Srivathsan Sivakumar <sri.skumar05@gmail.com>
Cc:     Manish Chopra <manishc@marvell.com>, GR-Linux-NIC-Dev@marvell.com,
        Coiby Xu <coiby.xu@gmail.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        netdev@vger.kernel.org, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: qlge: qlge_main.c: convert do-while loops to
 for loops
Message-ID: <20220609183419.GZ2168@kadam>
References: <YqIOp+cPXNxLAnui@Sassy>
 <20220609152653.GZ2146@kadam>
 <YqInZ/KNEJFN9kNS@Sassy>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YqInZ/KNEJFN9kNS@Sassy>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?KqrsuEwXCLKJmupd8QSea44h1IV64ziIdGm+L5M0k2RaVXRwe38XNWKkvbgA?=
 =?us-ascii?Q?aMiASDeJvyeJYTK0Jr5vgOyEfbgCpiJ92pb63mvoU5z/1j7SLXGCF6oSm3GJ?=
 =?us-ascii?Q?708sjDD6sQ8EBcV/7myP2e8Fie5UvMbJ5gF2V7qC/M57mN5LQGpaySpiwy9P?=
 =?us-ascii?Q?QMHz8+jgVuFwRwYuVE5Nq7B9n4Wre2Cs99tzT+H6pWp4Q+hvNUr+yxl52Kxk?=
 =?us-ascii?Q?DRisDALQx9ASSY7xy/YGbZbzt9v2+I5DcgxaydtlZCxHTd4lGqlQtPD+fHO8?=
 =?us-ascii?Q?+rDvSePw9RgECJ2E3UnX51/SIArZ787BgMeYqUXDCQVtocYDC95hpSOlDXIc?=
 =?us-ascii?Q?tP4Ozrg1oORpOCDHaqGapqu6SnNH+SBs02LJLwXGb8ZAKofRh7Y9ocufssL/?=
 =?us-ascii?Q?zhdqqaaySrPPkGrDPqCouXWIrsFCxyELNJ2wb804K0tbhiwM9Rdz+6XSbtdt?=
 =?us-ascii?Q?1eN/OufiUOdbdWlEvli2qd2I63jVRGbWC8piX+obDJxsD6iG/18nM+YyXoI0?=
 =?us-ascii?Q?vkGLHn6NwmTS8GjoIpp9iGJ/Q+7nM1dSb/VS5BAqSuTjKgCZ4X7mz5DHpaB/?=
 =?us-ascii?Q?12Ha0Nn+vKM7VU4L3d2JNwFVfstaFMfgwn6d89uQxXu1u1s/Q3lLEDmBcmok?=
 =?us-ascii?Q?0gixmm0gob9Pep8rGyEJacH/m8JxAzAkLj2jLPiQpQG2JNmCDIstn73F1M1x?=
 =?us-ascii?Q?Vt1CAGZUPDQEIyzYMHvJs+kBzwNw//u59e9vLpHxB0u+6IXUjVYKYwup6Y5K?=
 =?us-ascii?Q?zCwftsvDEqT77OBzl76LRLHc/otW3E3KTFlfycM9OBKsTh7GOw99iWrO3VKB?=
 =?us-ascii?Q?k9QwGUcS6UtOgF0tUipL2ssN7WsqJLvLGLrI7OX4Tnf4TrVVyR4Yirq9XM8W?=
 =?us-ascii?Q?VyRRtzHtFGbzOU9K+EXrlGUSeaQ7stKcHvgRIGJX/QqiUGnXbWRTMZcIl7v9?=
 =?us-ascii?Q?7vT3CA0YkpPl6jeE99mKim+DmSabK+gXYqUlt3VSYrSCGjDebyvz8QFG6FK9?=
 =?us-ascii?Q?X9MXqnAE6IwNlO9KVg72vwOQlO/zcB75T+TTvSj6TC/EHxntL2eQIKxgJu2S?=
 =?us-ascii?Q?551JOAsRqMeavQSTeXOm9K+lRveI8neRKJPsKVAHjdnHG5Yp71kkDzhrrznW?=
 =?us-ascii?Q?oLtvaBwjzS6YGfilsoIa9lve5rPg7TbedMECZM9MwcJ4MRb4oC5gJmWhUT24?=
 =?us-ascii?Q?FzkEjv8sp3aqtfkuxucurMdvYPyuoWXPvAoFnqtroq47LeUb7P4MxXD18sI1?=
 =?us-ascii?Q?EkR2bll8D06FmJen3/gew6Z80rhBGRzCgpWh3I/9wkV+Dg8GcyLMbpoUUchX?=
 =?us-ascii?Q?an+mCPeGSIJNlT7VQcwwOlLQKxFTb2UV4u9IY8LGwhw9eAZElXEXsBTZJ2Bj?=
 =?us-ascii?Q?bi5FrknyrI5S8qIBj8dQrbT9/4PCHHtsuQpDFNYLJqCljYIoLrkgM1jNDvLC?=
 =?us-ascii?Q?NKn57PT7s+Hi4Wi8ZfQLZwykq0CV/4XJApp5M+UWDoqLm7iXbZII/VZMfY+O?=
 =?us-ascii?Q?eyZpnsokM8O44UvEpHVdhmPxI05O7Z2zhY1cvHkipFUopzXH81dd2jenFBIE?=
 =?us-ascii?Q?Y3X/RUmngnjxiahxA87KJNZXDd2Nr+vszG4/lko1NsSWqhr6NDQnAwUgjVtu?=
 =?us-ascii?Q?b69b0Nv5Cfj1bpMmtZRut8upEtU279VhkoML/HzlamFAr+XUG3jN24SV7NM+?=
 =?us-ascii?Q?Ks6XYH8jZU0wdqpg/h+NE5HuHQUUnDseTJNfW+5B+tcAXVhUbPLDGtopODxf?=
 =?us-ascii?Q?fz8n1+0AuP+I6zFMek/bFPcmmm3/rl4=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e39d59c4-20cf-4cc9-b649-08da4a46b056
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jun 2022 18:34:29.6891
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: wLGyUFb6SBTe6+76TXhFiVVGOc1qWM0rAnHWRHU7eEELxDV4mbKyLu++4BDrKcvw2P426WgjPLS05TN1upOuAiaZgFtGMoton/NGtikPO2A=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4431
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.874
 definitions=2022-06-09_14:2022-06-09,2022-06-09 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 bulkscore=0 spamscore=0
 phishscore=0 mlxscore=0 adultscore=0 suspectscore=0 mlxlogscore=840
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2204290000
 definitions=main-2206090067
X-Proofpoint-ORIG-GUID: aespcrAil7QYI4fKpFL3JsMthPi8wEqK
X-Proofpoint-GUID: aespcrAil7QYI4fKpFL3JsMthPi8wEqK
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Jun 09, 2022 at 01:01:27PM -0400, Srivathsan Sivakumar wrote:
> On Thu, Jun 09, 2022 at 06:26:53PM +0300, Dan Carpenter wrote:
> > On Thu, Jun 09, 2022 at 11:15:51AM -0400, Srivathsan Sivakumar wrote:
> > > diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
> > > index 8c35d4c4b851..308e8b621185 100644
> > > --- a/drivers/staging/qlge/qlge_main.c
> > > +++ b/drivers/staging/qlge/qlge_main.c
> > > @@ -3006,13 +3006,13 @@ static int qlge_start_rx_ring(struct qlge_adapter *qdev, struct rx_ring *rx_ring
> > >  		cqicb->flags |= FLAGS_LL;	/* Load lbq values */
> > >  		tmp = (u64)rx_ring->lbq.base_dma;
> > >  		base_indirect_ptr = rx_ring->lbq.base_indirect;
> > > -		page_entries = 0;
> > > -		do {
> > > -			*base_indirect_ptr = cpu_to_le64(tmp);
> > > -			tmp += DB_PAGE_SIZE;
> > > -			base_indirect_ptr++;
> > > -			page_entries++;
> > > -		} while (page_entries < MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN));
> > > +
> > > +		for (page_entries = 0; page_entries <
> > > +			MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); page_entries++) {
> > > +				*base_indirect_ptr = cpu_to_le64(tmp);
> > > +				tmp += DB_PAGE_SIZE;
> > > +				base_indirect_ptr++;
> > > +		}
> > 
> > It's better than the original, but wouldn't it be better yet to write
> > something like this (untested):
> > 
> > 		for (i = 0; i <	MAX_DB_PAGES_PER_BQ(QLGE_BQ_LEN); i++)
> > 			base_indirect_ptr[i] = cpu_to_le64(tmp + (i * DB_PAGE_SIZE));
> > 
> > Same with the other as well, obviously.
> > 
> > regards,
> > dan carpenter
> > 
> 
> Hello Dan,
> 
> Thanks for your input
> 
> wouldn't base_indirect_ptr point at a different endian value if tmp is
> added with (i * DB_PAGE_SIZE)?

tmp is cpu endian so we can do math on it.  Then we convert the result
to le64.  This is how it works before and after.  What isn't allowed
(doesn't make sense) is to do math on endian data so "cpu_to_le64(tmp) +
i * DB_PAGE_SIZE" is wrong.

Sparse can detect endian bugs like that:
https://lwn.net/Articles/205624/

regards,
dan carpenter