netdev - RE: [PATCH iwl-next v2] ixgbe: Convert ret val type from s32 to int

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 for Android: free password hash cracker in your pocket

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

Date: Wed, 10 Jan 2024 11:51:53 +0000
From: "Jagielski, Jedrzej" <jedrzej.jagielski@...el.com>
To: "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Keller, Jacob E"
	<jacob.e.keller@...el.com>
Subject: RE: [PATCH iwl-next v2] ixgbe: Convert ret val type from s32 to int

From: Nguyen, Anthony L <anthony.l.nguyen@...el.com> 
Sent: Friday, January 5, 2024 10:50 PM

>On 1/5/2024 2:31 AM, Jedrzej Jagielski wrote:
>> Currently big amount of the functions returning standard error codes
>> are of type s32. Convert them to regular ints as typdefs here are
>> not necessary to return standard error codes.
>> 
>> Suggested-by: Jacob Keller <jacob.e.keller@...el.com>
>> Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
>> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@...el.com>
>> ---
>> v2: fix smatch warnings,
>
>These changes aren't mentioned in the commit message, however, it should 
>probably be in it's own commit to keep the changes separate and easier 
>to see/review (since they pre-existed this patch).

OK, i agree it's better to create a separate commit for that

>
>Also, not seeing changes/fixes to the second warning [1]
>
>New smatch warnings:
>...
>drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c:3130 
>ixgbe_enter_lplu_t_x550em() warn: missing error code? 'status'
>
>> alter the commit msg
>> ---
>
>...
>
>> @@ -93,11 +93,11 @@ static s32 ixgbe_get_invariants_82598(struct ixgbe_hw *hw)
>>    *  not known.  Perform the SFP init if necessary.
>>    *
>>    **/
>> -static s32 ixgbe_init_phy_ops_82598(struct ixgbe_hw *hw)
>> +static int ixgbe_init_phy_ops_82598(struct ixgbe_hw *hw)
>>   {
>>   	struct ixgbe_mac_info *mac = &hw->mac;
>>   	struct ixgbe_phy_info *phy = &hw->phy;
>> -	s32 ret_val;
>> +	int ret_val;
>>   	u16 list_offset, data_offset;
>
>Should we RCT what we're touching? (many other instances in the patch as 
>well)

I am not sure if that's still in the scope of that patch

>
>...
>
>> @@ -1866,9 +1866,9 @@ static s32 ixgbe_enable_rx_dma_82599(struct ixgbe_hw *hw, u32 regval)
>>    *  Returns IXGBE_ERR_EEPROM_VERSION if the FW is not present or
>>    *  if the FW version is not supported.
>>    **/
>> -static s32 ixgbe_verify_fw_version_82599(struct ixgbe_hw *hw)
>> +static int ixgbe_verify_fw_version_82599(struct ixgbe_hw *hw)
>>   {
>> -	s32 status = IXGBE_ERR_EEPROM_VERSION;
>> +	int status = IXGBE_ERR_EEPROM_VERSION;
>
>Does IXGBE_ERR_EEPROM_VERSION exist anymore after your other patch? [2]

Sure, need to rebase it

>
>>   	u16 fw_offset, fw_ptp_cfg_offset;
>>   	u16 offset;
>>   	u16 fw_version = 0;
>
>...
>
>> @@ -2062,12 +2062,12 @@ static s32 ixgbe_reset_pipeline_82599(struct ixgbe_hw *hw)
>>    *  Performs byte read operation to SFP module's EEPROM over I2C interface at
>>    *  a specified device address.
>>    **/
>> -static s32 ixgbe_read_i2c_byte_82599(struct ixgbe_hw *hw, u8 byte_offset,
>> +static int ixgbe_read_i2c_byte_82599(struct ixgbe_hw *hw, u8 byte_offset,
>>   				     u8 dev_addr, u8 *data)
>>   {
>>   	u32 esdp;
>> -	s32 status;
>> -	s32 timeout = 200;
>> +	int status;
>> +	int timeout = 200;
>
>unrelated change (at least a few others in this patch as well)

Thank you for pointing this out, need to take a look on that once again

>
>>   
>>   	if (hw->phy.qsfp_shared_i2c_bus == true) {
>>   		/* Acquire I2C bus ownership. */
>> @@ -2116,12 +2116,12 @@ static s32 ixgbe_read_i2c_byte_82599(struct ixgbe_hw *hw, u8 byte_offset,
>>    *  Performs byte write operation to SFP module's EEPROM over I2C interface at
>>    *  a specified device address.
>>    **/
>> -static s32 ixgbe_write_i2c_byte_82599(struct ixgbe_hw *hw, u8 byte_offset,
>> +static int ixgbe_write_i2c_byte_82599(struct ixgbe_hw *hw, u8 byte_offset,
>>   				      u8 dev_addr, u8 data)
>>   {
>>   	u32 esdp;
>> -	s32 status;
>> -	s32 timeout = 200;
>> +	int status;
>> +	int timeout = 200;
>>   
>>   	if (hw->phy.qsfp_shared_i2c_bus == true) {
>>   		/* Acquire I2C bus ownership. */
>
>Thanks,
>Tony
>
>[1] 
>https://lore.kernel.org/intel-wired-lan/08d8b75e-af80-438b-8006-9121b8444f49@moroto.mountain/
>[2] 
>https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit?id=5795f533f30a80aa0473652876296ebc9129e33a