Re: [Outreachy kernel] [PATCH] staging: vt6655: Rename byPreambleType field

[Karolina Drobnik <karolinadrobnik@gmail.com>]

Hi,

On Thu, 2021-10-14 at 17:26 +0200, Julia Lawall wrote:
> On Thu, 14 Oct 2021, Karolina Drobnik wrote:
>
> > Drop Hungarian notation prefix in `byPreambleType` member of
> > struct vnt_private. Change it to use snake case.
>
> Are you sure that it is an instance of Hungarian notation?  Could it
> be that "by" is meant to be the word "by"?

I saw that functions and parameters use it[1] so I deduced that "by"
might stand for byte. If this is not a case, I can revert that change
and leave `by_preamble_type` name. What do you think?

> I have the imrepssion from a
> quick look through the code that it should be a boolean. 

Hmm, it really looks like it. In `vnt_bss_info_changed` definition we
use true/false values but in `bb_get_frame_time` we have such a line:

```
if (byPreambleType == 1)     /* Short */
```

I'm not quite sure to what this "short" might refer to.

> Maybe it  would be helpful to give it boolean value, 
> eg true instead of 1.

This is a good idea but I'm not sure if it's only used as a boolean. 
I could add this change but I'd like to test it more than just by
loading/unloading the module. Is there any way of testing drivers
without an actual hardware?


Thanks,
Karolina

----------------------------------------
[1] - For example in rxtx.c, lines 154-161:
```
static
unsigned int
s_uGetTxRsvTime(                     /* s - static, u - unsigned */
        struct vnt_private *pDevice, /* p - pointer */
        unsigned char byPktType,     /* by - byte */
        unsigned int cbFrameLength,  /* not sure */
        unsigned short wRate,        /* w - word */
        bool bNeedAck                /* b - boolean */
)
```