Re: [PATCH] staging: rtl8723bs: remove unnecessary type encoding in variable names
Samuel Abraham <[email protected]> Sat, 29 Mar 2025 15:09:49 +0100
| Newsgroups | dev.linux.lists.outreachy,dev.linux.lists.linux-staging,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CADYq+fYizY_eRozZkKYzsD00biQMTGfhOcuqiQ0RzXApXiAF6A@mail.gmail.com> |
On Sat, Mar 29, 2025 at 12:13 PM Julia Lawall <[email protected]> wrote: > > > > On Sat, 29 Mar 2025, Abraham Samuel Adekunle wrote: > > > type encoding in variable names are not a standard in Linux kernel coding > > style. > > > > Remove redundant type prefixes (e.g, `b`, `p`) in variable names, > > as explicit type encoding is not necessary in Linux kernel code which > > uses type definitions rather than variable name prefixes > > You seem to have also gotten rid of capitalization. Hello Julia, thank you for your review Yes, I should have added that to my commit message. Thank you. > It's also not clear how you have chosen which variables to update. Mostly > it seems to be pDM_Odm, but there is also pRFCalibrateInfo in some > comments. But you haven't updated eg bMaskDWord. I chose to update the boolean and pointer variables which have been declared in the source files I was working on. pDM_Odm, declared in the source file, is a pointer of type struct dm_odm_t, which has been declared in a header file, so altering the pointer name would have no compiler errors since it is declared in the source file I modified. Some function prototypes have been declared in header files, so altering their names in their definition in the files I was editing would result in compiler errors too if the headers were not modified. I could have modified the variables in those header files too in drivers/staging/rtl8723bs/include but I was not sure how many files would be affected by the change and how long my patch would be, considering the three files I modified already made my patch over 3000 lines long. RFCalibrateInfo(without the p) is a pointer that is a member of the struct dm_odm_t, which has been declared in the header file, so altering that in the source file would result in compiler errors too, since the header file was not modified in drivers/staging/rtl8723bs/include/ > > I don't know what the r represents in rOFDM0_XATxIQImbalance. The bMaskWord is a macro defined in the drivers/staging/rtl8723bs/include/Hal8192CPhyReg.h as `#define bMaskDWord 0xffffffff and also rOFDM0_XATxIQImbalance is a macro defined as `#define rOFDM0_XATxIQImbalance 0xc80` in the header file; these two values are not boolean values and are also declared in the header, so altering them in the source files will result in compiler errors However, other Boolean variables declared in the source files were modified. > > julia > > > > > > > - struct odm_rf_cal_t *pRFCalibrateInfo = &pDM_Odm->RFCalibrateInfo; > > + struct odm_rf_cal_t *rf_calibrate_info = &dm_odm->RFCalibrateInfo; Example of RFCalibrateInfo (with the p ) being declared as a pointer of type struct odm_rf_cal_t and also a member of the struct dm_odm_t So the declared variable(with the p) was modified, but the member(without the p) was not modified. With these, please what suggestions do you have? Thanks Adekunle