linux-kernel - Re: [PATCH] staging: rtl8723bs: remove unnecessary type encoding in variable names

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.22.394.2503291643170.58211@hadrien>
Date: Sat, 29 Mar 2025 16:43:33 +0100 (CET)
From: Julia Lawall <julia.lawall@...ia.fr>
To: Samuel Abraham <abrahamadekunle50@...il.com>
cc: outreachy@...ts.linux.dev, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
    linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: rtl8723bs: remove unnecessary type encoding in
 variable names



On Sat, 29 Mar 2025, Samuel Abraham wrote:

> On Sat, Mar 29, 2025 at 3:20 PM Julia Lawall <julia.lawall@...ia.fr> wrote:
> >
> >
> >
> > On Sat, 29 Mar 2025, Samuel Abraham wrote:
> >
> > > On Sat, Mar 29, 2025 at 12:13 PM Julia Lawall <julia.lawall@...ia.fr> 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.
> >
> > OK.  It shows how confusing the code is.  Normally in the Linux kernel
> > things defined with #define are fully capitalized, unless they refer to
> > some name from a hardware spec.
> >
> > I'm a little surprised that there isn't some generic macro defined as
> > 0xffffffff in the Linux kernel, but indeed I don't see one, and I see lots
> > of masks being defined as that.
> >
> > julia
> >
> Yes.
> Should I make any modifications to the patch?

No.

julia