[PATCH v6 0/2] staging: rtl8723bs: Fix error handling in _rtw_pktfile_read()

Minu Jin posted 2 patches 2 weeks, 2 days ago
There is a newer version of this series
[PATCH v6 0/2] staging: rtl8723bs: Fix error handling in _rtw_pktfile_read()
Posted by Minu Jin 2 weeks, 2 days ago
This series improves error handling in _rtw_pktfile_read() and cleans up
the code style to comply with kernel standards.

In v6, I have reorganized the patches into a series of two to satisfy
both Greg and Dan's feedback:

    1. The first patch combines the logic change and caller updates.
       The function change and the caller updates must be in the same
       patch. If they are separated, the code will not work correctly 
       or will cause errors at that specific point in the history.

    2. The second patch focuses purely on code style cleanup (changing uint
       to unsigned int) as requested by Andy Shevchenko.

Regarding the logic change in _rtw_pktfile_read():

    The original code used a ternary operator to read whatever data was 
    available, even if it was less than requested. This could lead to 
    callers processing incomplete data without knowing it.

    I have changed this to return -EINVAL when the remaining data is insufficient. 
    This is safer because most callers expect the exact amount of data and 
    should not proceed with a partial read.

Changes in v6:
    - Reorganized into a 2-patch series to maintain git bisect safety.
    - Combined function logic changes with caller updates into Patch 1.
    - Separated style cleanup (uint -> unsigned int) into Patch 2.

-- 
2.43.0
Re: [PATCH v6 0/2] staging: rtl8723bs: Fix error handling in _rtw_pktfile_read()
Posted by Greg KH 2 weeks, 1 day ago
On Fri, Jan 23, 2026 at 02:41:23AM +0900, Minu Jin wrote:
> This series improves error handling in _rtw_pktfile_read() and cleans up
> the code style to comply with kernel standards.
> 
> In v6, I have reorganized the patches into a series of two to satisfy
> both Greg and Dan's feedback:
> 
>     1. The first patch combines the logic change and caller updates.
>        The function change and the caller updates must be in the same
>        patch. If they are separated, the code will not work correctly 
>        or will cause errors at that specific point in the history.
> 
>     2. The second patch focuses purely on code style cleanup (changing uint
>        to unsigned int) as requested by Andy Shevchenko.
> 
> Regarding the logic change in _rtw_pktfile_read():
> 
>     The original code used a ternary operator to read whatever data was 
>     available, even if it was less than requested. This could lead to 
>     callers processing incomplete data without knowing it.
> 
>     I have changed this to return -EINVAL when the remaining data is insufficient. 
>     This is safer because most callers expect the exact amount of data and 
>     should not proceed with a partial read.
> 
> Changes in v6:
>     - Reorganized into a 2-patch series to maintain git bisect safety.
>     - Combined function logic changes with caller updates into Patch 1.
>     - Separated style cleanup (uint -> unsigned int) into Patch 2.

Where is the list of changes for previous versions?

Also, was this tested on real hardware for this device?  As you are
changing the main data path, that should be required here.

thanks,

greg k-h
Re: [PATCH v6 0/2] staging: rtl8723bs: Fix error handling in _rtw_pktfile_read()
Posted by Minu Jin 2 weeks, 1 day ago
On Fri, Jan 23, 2026 at 07:25:07AM +0100, Greg KH wrote:
> On Fri, Jan 23, 2026 at 02:41:23AM +0900, Minu Jin wrote:
> > This series improves error handling in _rtw_pktfile_read() and cleans up
> > the code style to comply with kernel standards.
> > 
> > In v6, I have reorganized the patches into a series of two to satisfy
> > both Greg and Dan's feedback:
> > 
> >     1. The first patch combines the logic change and caller updates.
> >        The function change and the caller updates must be in the same
> >        patch. If they are separated, the code will not work correctly 
> >        or will cause errors at that specific point in the history.
> > 
> >     2. The second patch focuses purely on code style cleanup (changing uint
> >        to unsigned int) as requested by Andy Shevchenko.
> > 
> > Regarding the logic change in _rtw_pktfile_read():
> > 
> >     The original code used a ternary operator to read whatever data was 
> >     available, even if it was less than requested. This could lead to 
> >     callers processing incomplete data without knowing it.
> > 
> >     I have changed this to return -EINVAL when the remaining data is insufficient. 
> >     This is safer because most callers expect the exact amount of data and 
> >     should not proceed with a partial read.
> > 
> > Changes in v6:
> >     - Reorganized into a 2-patch series to maintain git bisect safety.
> >     - Combined function logic changes with caller updates into Patch 1.
> >     - Separated style cleanup (uint -> unsigned int) into Patch 2.
> 
> Where is the list of changes for previous versions?
> 
> Also, was this tested on real hardware for this device?  As you are
> changing the main data path, that should be required here.
> 
> thanks,
> 
> greg k-h

Hi Greg,

Regarding hardware testing:
I don't have access to hardware with this chipset. However,
I have
    - Reviewed all call sites carefully
    - Verified the code compiles without warnings
    - Ran smatch static analysis with no issues found

If anyone has the hardware and could test this series, I would
appreciate it.

Regarding the change log history:
Changes in v6:
    - Reorganized into a 2-patch series to maintain git bisect safety
      (addressed Dan Carpenter's feedback).
    - Combined function logic changes with caller updates into Patch 1.
    - Separated style cleanup (uint -> unsigned int) into Patch 2.

Changes in v5:
    - Split the patch into a 3-patch series (suggested by Greg KH).
    - Patch 1: Refactored return type to 'int'.
    - Patch 2: Added error checks to call sites.
    - Patch 3: Implemented -EINVAL logic.
    (Note: Superseded by v6 to fix git bisect issues).

Changes in v4:
    - Modified _rtw_pktfile_read() to return -EINVAL if data is
      insufficient (following Greg KH's hint).
    - Replaced 'uint' with 'unsigned int' (suggested by Andy Shevchenko).
    - Removed redundant local variable 'len'.

Changes in v3:
    - Updated set_qos() from void to int (suggested by Greg KH, Dan Carpenter).
    - Fixed coding style issues (suggested by Andy Shevchenko).

Changes in v2:
    - Added check for skb_copy_bits() return value.

Thanks,
Minu