From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758105AbcLTO5V convert rfc822-to-8bit (ORCPT ); Tue, 20 Dec 2016 09:57:21 -0500 Received: from mga09.intel.com ([134.134.136.24]:26364 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752252AbcLTO5T (ORCPT ); Tue, 20 Dec 2016 09:57:19 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,379,1477983600"; d="scan'208";a="44951536" From: "Hammond, John" To: Dan Carpenter , Bruce Korb CC: James Simmons , "devel@driverdev.osuosl.org" , "Dilger, Andreas" , Kees Cook , Greg Kroah-Hartman , Linux Kernel Mailing List , "Faccini, Bruno" , "Drokin, Oleg" , Vitaly Fertman , "Liu, Emoly" , "Lustre Development List" Subject: RE: [PATCH] staging: lustre: ldlm: use designated initializers Thread-Topic: [PATCH] staging: lustre: ldlm: use designated initializers Thread-Index: AQHSWAEE5iDKjggwJEuUl1tnxeqh4qEP/mwAgAAG8gCAAPEMAP//+yrg Date: Tue, 20 Dec 2016 14:57:17 +0000 Message-ID: References: <20161217010045.GA140343@beast> <20161220071034.GL8176@mwanda> In-Reply-To: <20161220071034.GL8176@mwanda> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzllNDM1ZDItNDZmZS00ZWRiLWE5NTYtODk5OGQyM2Q5MzBiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6ImdmOVNrS1wvMUt5NEdNUzF1OHJPSEJNU3JrcWF4WDJqczg1eDNyYjhjT0EwPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [10.1.200.106] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On Mon, Dec 19, 2016 at 08:47:50AM -0800, Bruce Korb wrote: > > On Mon, Dec 19, 2016 at 8:22 AM, James Simmons > > >> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c > > >> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c > > >> @@ -143,7 +143,7 @@ static int ldlm_process_flock_lock(struct ldlm_lock > *req, __u64 *flags, > > >> int added = (mode == LCK_NL); > > >> int overlaps = 0; > > >> int splitted = 0; > > >> - const struct ldlm_callback_suite null_cbs = { NULL }; > > >> + const struct ldlm_callback_suite null_cbs = { }; > > >> > > >> CDEBUG(D_DLMTRACE, > > >> "flags %#llx owner %llu pid %u mode %u start %llu end > > >> %llu\n", > > > > > > Nak. Filling null_cbs with random data is a bad idea. If you look at > > > ldlm_lock_create() where this is used you have > > > > > > if (cbs) { > > > lock->l_blocking_ast = cbs->lcs_blocking; > > > lock->l_completion_ast = cbs->lcs_completion; > > > lock->l_glimpse_ast = cbs->lcs_glimpse; } > > > > > > Having lock->l_* point to random addresses is a bad idea. > > > What really needs to be done is proper initialization of that > > > structure. A bunch of patches will be coming to address this. > > > > I'm not understanding the effect of the original difference. If you > > specify any initializer, then all fields not specified are filled with > > zero bits. Any pointers are, perforce, NULL. That should make both "{ > > NULL }" and "{}" equivalent. > > They are equivalent, yes, but people want to use a GCC plugin that randomizes > struct layouts for internal structures and the plugin doesn't work when you use > struct ordering to initialize the struct. The plugin requires that you use > designated intializers. "{ NULL }" is valid ISO C, but unfortunately "{}" is not. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hammond, John Date: Tue, 20 Dec 2016 14:57:17 +0000 Subject: [lustre-devel] [PATCH] staging: lustre: ldlm: use designated initializers In-Reply-To: <20161220071034.GL8176@mwanda> References: <20161217010045.GA140343@beast> <20161220071034.GL8176@mwanda> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Dan Carpenter , Bruce Korb Cc: James Simmons , "devel@driverdev.osuosl.org" , "Dilger, Andreas" , Kees Cook , Greg Kroah-Hartman , Linux Kernel Mailing List , "Faccini, Bruno" , "Drokin, Oleg" , Vitaly Fertman , "Liu, Emoly" , Lustre Development List > On Mon, Dec 19, 2016 at 08:47:50AM -0800, Bruce Korb wrote: > > On Mon, Dec 19, 2016 at 8:22 AM, James Simmons > > >> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c > > >> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_flock.c > > >> @@ -143,7 +143,7 @@ static int ldlm_process_flock_lock(struct ldlm_lock > *req, __u64 *flags, > > >> int added = (mode == LCK_NL); > > >> int overlaps = 0; > > >> int splitted = 0; > > >> - const struct ldlm_callback_suite null_cbs = { NULL }; > > >> + const struct ldlm_callback_suite null_cbs = { }; > > >> > > >> CDEBUG(D_DLMTRACE, > > >> "flags %#llx owner %llu pid %u mode %u start %llu end > > >> %llu\n", > > > > > > Nak. Filling null_cbs with random data is a bad idea. If you look at > > > ldlm_lock_create() where this is used you have > > > > > > if (cbs) { > > > lock->l_blocking_ast = cbs->lcs_blocking; > > > lock->l_completion_ast = cbs->lcs_completion; > > > lock->l_glimpse_ast = cbs->lcs_glimpse; } > > > > > > Having lock->l_* point to random addresses is a bad idea. > > > What really needs to be done is proper initialization of that > > > structure. A bunch of patches will be coming to address this. > > > > I'm not understanding the effect of the original difference. If you > > specify any initializer, then all fields not specified are filled with > > zero bits. Any pointers are, perforce, NULL. That should make both "{ > > NULL }" and "{}" equivalent. > > They are equivalent, yes, but people want to use a GCC plugin that randomizes > struct layouts for internal structures and the plugin doesn't work when you use > struct ordering to initialize the struct. The plugin requires that you use > designated intializers. "{ NULL }" is valid ISO C, but unfortunately "{}" is not.