Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 4 May 2022 12:33:48 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Jaehee <jhpark1013@gmail.com>
Cc:     Kalle Valo <kvalo@kernel.org>,
        =?iso-8859-1?B?Suly9G1l?= Pouiller <jerome.pouiller@silabs.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-staging@lists.linux.dev,
        Outreachy Linux Kernel <outreachy@lists.linux.dev>,
        Stefano Brivio <sbrivio@redhat.com>
Subject: Re: [PATCH] wfx: use container_of() to get vif
Message-ID: <20220504093347.GB4009@kadam>
References: <20220418035110.GA937332@jaehee-ThinkPad-X1-Extreme>
 <87y200nf0a.fsf@kernel.org>
 <CAA1TwFCOEEwnayexnJin8T=Fc2HEgHC9jyfj5HxfiWybjUi9GA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAA1TwFCOEEwnayexnJin8T=Fc2HEgHC9jyfj5HxfiWybjUi9GA@mail.gmail.com>
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: =?utf-8?B?bkZnOVJWc3hCSnltaVlwUURKR2NwNE94Y1JLbGhyTHdtU00wZ0oxd25FQSs4?=
 =?utf-8?B?TE9XSUJxb2tETlFjcGFlVExHRUZGTFNGcy82QVlLZDhPZWhEQXF0SVBhY0Fv?=
 =?utf-8?B?bmNBNG9IbXBPbUk1Z1RtdmFWb3Fuc1hHMjB1Z2xZajFzRm4rZGlmdk5MaXdU?=
 =?utf-8?B?NWt5bDY3UC9TdVUvaDNjWmVEaWMrVFBiY3p1NXpFYytRU2lpSkVscVIxWFF1?=
 =?utf-8?B?Z0V2dTF2cm1YZUljZ2IxS2p6WmhONEdWdWtqOHdZUkJsalBKV2o5ZUpGVk04?=
 =?utf-8?B?b2FiN1hzN2NPQTcyenkvOEpBUlhoRmtGOFJKWlRvS2FUYUJHS2g2eDlmSjhz?=
 =?utf-8?B?K0F3eVRSeThWWitTcTliZ2lYWVFJSlpURFlvZy9JZ3d4WlMwK3BueGJFNzZm?=
 =?utf-8?B?Ny9wOG9tenNrUjFYa0dubWl3bEkrQWRPdUZabGtkakIxeVlhbTQxdUU4U1h0?=
 =?utf-8?B?WmVlNUVYd3FrYXg2UlBVZTg0UERkakVQTVZENWgvRHF4UG1KYXFSZEVyQmVx?=
 =?utf-8?B?KzdHWGxBVzVzSkpKYm1tWFltZ0d6emRpcjlYd1pBVVRyUzF0YmVKd0ZteHpp?=
 =?utf-8?B?REZyZDRMc3ZvWEg4dldvZFpJKzdGUnNieTJ0cWt2MUZ0SmppUE11b3lVS0hs?=
 =?utf-8?B?OGI4M095S0VMbHhxTndSb1BDR2U1cmtDRitwamxXVGRWaUl3eC9aWGdaTG5W?=
 =?utf-8?B?VU9lWSt0MnYrRE9HWnI2eklPR3pPU3hOVEVGbm5hT2xRa0JmUEJVV1dCUHYv?=
 =?utf-8?B?dU0rVEZtQlZzdGM4UFpoMm8wMWpUQnp0SHo4c0tibmFJUHlITWZvaXMzSmt3?=
 =?utf-8?B?eitMaWpXbTdWbC9vdWJiWUU1N0lRSC9KK0VoRGRSczhRK0UrcFZxZHl1SHRi?=
 =?utf-8?B?UVRFZTlRdHQ5RFlYK2hEQUtyVDZDOS9aOU1La2t3V016UUtqNTRldzNNbDlU?=
 =?utf-8?B?NzgrRnVPREp1ZGF1aTBId00rT3U4VWRWRFJLemY1bmk5UHpZVWc1OFNOa1dZ?=
 =?utf-8?B?T3VEcVZ4ZStOU2hkS0ErZ3VGaVE4QTZPbzZOZUJNL1kzNWliTVlDeC9YTkhx?=
 =?utf-8?B?VUxJNzdWQ2RiSjlhbFZYUFZJem94Rm9PcThtd3dUdzdkOHdZNVZzWTN2RnFN?=
 =?utf-8?B?cVMvTWFUZy9SUEN1L0Vsam00RlRHYWRQNTBtVzhZVUk5c0NiSnd2Z2RpN0Vu?=
 =?utf-8?B?VEhIL1dGTk80bmx0dnVuUmV4bTJkS01obm1EOStpdzZSdnY3cjBhR1htNzZ3?=
 =?utf-8?B?d2dWb3pIYU1KRGdvVHJMeWJBZzN3RGxwV0hPdDFuencvNmtWY3hPY2NOZngz?=
 =?utf-8?B?VmdVaGpNM1lLaTk2ZWE3VVRBazBYT3o3NFBudVdUbnhVOHFqVUowVGtTbm94?=
 =?utf-8?B?MFVjQ2Naa2NUcFZ4dmVjcDhFOEVxL0wvM3ZsOWZ3akhhMlBaWHZPMmdwZlhs?=
 =?utf-8?B?d254NENMS1NPVURTREV1NkJDRTk5bk9ZcDd6aEw1bUFkTzB1K25WQVQ4SW5m?=
 =?utf-8?B?S2RibXNpUFQyZjJxckswZTY1cm1DdHd3ZUZyMlkzTFZNLzlNU1c0OUpHY09T?=
 =?utf-8?B?V05TaWc3Z1RjaDNTL2dlWk5PaDFPYktTdlBadjUwaTcrQm5Sb1hnVVVaU2cy?=
 =?utf-8?B?cnhFaURmZUZtdThFYkExa2htYzI4eDdCck9rempFYnBtcG5BS0NselJ1Z3du?=
 =?utf-8?B?UlQ4MktMVTBQb1N6YjkxY0RMQU1vVHk0czU3dkRrSzR0dFlvbXR1dUJtdE1C?=
 =?utf-8?B?NkpkOWRHNEFqVG9WanF3WS92TkI0TDJuN1N4akdJQWovRnlRYW0xRkpzZXVL?=
 =?utf-8?B?ZHA5RlVoVkRSR3NEclZxK3Q4alZ0dndzanJvcW00R0QyQnhYMVhJVnM2UlFG?=
 =?utf-8?B?Zkg4QXNPczMxUzBXdk9oczE4NFMzUnlEa0xSTWpjdEpKd0tVODczT0VKSmJ3?=
 =?utf-8?B?VUpMM2x1MmxtbW5ZKzUrRnZNUWV5SDB6VzNBeGVLUkszWGV5SmpZWEJINUtT?=
 =?utf-8?B?WndTZnUzSHo2OUxpNVFtV0VEdkRqTElHWk9DREEreXhKRWlvenBSeUlGdVNZ?=
 =?utf-8?B?ZzZEWHFmbVU2K0xTcy96N3d6L21DK1g3SmFFajI3QUh3b2haU3F6aXQ0VlVn?=
 =?utf-8?B?M1RkVlNwM3E4aFgrNkdBYVp3VGIwdHI4dWtKbmVPM3I2bjAyN0VMNDRFRDda?=
 =?utf-8?B?cUNnL2tMM1FDemg5Vmw5NVN1STNRdkdRZGQ3Vnczc3RmNVpTUU1PQmZEZTll?=
 =?utf-8?B?ZGJlNmZGTjhIY3htYW9yTS9ndUxlWWZuUHpGdytkSTF2MXlleEJXck9HWE9h?=
 =?utf-8?B?RnBoaitSNFBDUDJLV1E4OUROYVAxMTBiNjdTVlo5eE9lK0lFKzNsRTAzMGZW?=
 =?utf-8?Q?XFQDPT4dI/fiwjY0=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9705d73e-e691-49ce-8976-08da2db137fb
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 May 2022 09:34:00.3507
 (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: rffes3zVLj/RNKmjYSVi0nfrk4z7BCDIWlsJPaIdQ3lUG2Qx4esRE+bE3Docx5QrxOn5/ahw9rY6Ip5vQXLuVzMkdI0iRRLl6nkLPuMi1yw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB3834
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-05-04_03:2022-05-02,2022-05-04 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 spamscore=0
 malwarescore=0 adultscore=0 suspectscore=0 mlxscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2205040063
X-Proofpoint-ORIG-GUID: 06-oWn5R3Jfwl1Qf_67eKQLe-kYsExD0
X-Proofpoint-GUID: 06-oWn5R3Jfwl1Qf_67eKQLe-kYsExD0
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,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE 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-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org

On Mon, May 02, 2022 at 02:10:07PM -0400, Jaehee wrote:
> On Wed, Apr 20, 2022 at 7:58 AM Kalle Valo <kvalo@kernel.org> wrote:
> >
> > Jaehee Park <jhpark1013@gmail.com> writes:
> >
> > > Currently, upon virtual interface creation, wfx_add_interface() stores
> > > a reference to the corresponding struct ieee80211_vif in private data,
> > > for later usage. This is not needed when using the container_of
> > > construct. This construct already has all the info it needs to retrieve
> > > the reference to the corresponding struct from the offset that is
> > > already available, inherent in container_of(), between its type and
> > > member inputs (struct ieee80211_vif and drv_priv, respectively).
> > > Remove vif (which was previously storing the reference to the struct
> > > ieee80211_vif) from the struct wfx_vif, define a function
> > > wvif_to_vif(wvif) for container_of(), and replace all wvif->vif with
> > > the newly defined container_of construct.
> > >
> > > Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> >
> > [...]
> >
> > > +static inline struct ieee80211_vif *wvif_to_vif(struct wfx_vif *wvif)
> > > +{
> > > +     return container_of((void *)wvif, struct ieee80211_vif, drv_priv);
> > > +}
> >
> > Why the void pointer cast? Avoid casts as much possible.
> >
> 
> Hi Kalle,
> 
> Sorry for the delay in getting back to you about why the void pointer
> cast was used.
> 
> In essence, I'm taking private data with a driver-specific pointer
> and that needs to be resolved back to a generic pointer.
> 
> The private data (drv_priv) is declared as a generic u8 array in struct
> ieee80211_vif, but wvif is a more specific type.
> 
> I wanted to also point to existing, reasonable examples such as:
> static void iwl_mvm_tcm_uapsd_nonagg_detected_wk(struct work_struct *wk)
> {
>         struct iwl_mvm *mvm;
>         struct iwl_mvm_vif *mvmvif;
>         struct ieee80211_vif *vif;
> 
>         mvmvif = container_of(wk, struct iwl_mvm_vif,
>                               uapsd_nonagg_detected_wk.work);
>         vif = container_of((void *)mvmvif, struct ieee80211_vif, drv_priv);
> 
> in drivers/net/wireless$ less intel/iwlwifi/mvm/utils.c, which does the
> same thing.
> 
> There are fifteen of them throughout:

The cast is fine, but this email is frustrating.

It sounds like you are saying that you copied it from other code and
that's not a good answer...  :/  It's easiest if you just copy and paste
the build error and we can figure out why the cast is need for our
selves...

drivers/net/wireless/silabs/wfx/data_rx.c: In function ‘wvif_to_vif’:
./include/linux/build_bug.h:78:41: error: static assertion failed: "pointer type mismatch in container_of()"
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                         ^~~~~~~~~~~~~~
./include/linux/build_bug.h:77:34: note: in expansion of macro ‘__static_assert’
   77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
      |                                  ^~~~~~~~~~~~~~~
./include/linux/container_of.h:19:9: note: in expansion of macro ‘static_assert’
   19 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
      |         ^~~~~~~~~~~~~
drivers/net/wireless/silabs/wfx/data_rx.c:20:16: note: in expansion of macro ‘container_of’
   20 |         return container_of(wvif, struct ieee80211_vif, drv_priv);
      |                ^~~~~~~~~~~~

regards,
dan carpenter