Risk of rte_ether_addr_copy() causing bugs

Risk of rte_ether_addr_copy() causing bugs

[Morten Brørup]

Unlike memcpy() and other copy functions, rte_ether_addr_copy() takes the destination as the second parameter.

Not following well established conventions adds a high risk of causing bugs in the applications/libraries/drivers; it is likely that developers expect copy() functions to take parameters in the usual memcpy() order, and pass the parameters to rte_ether_addr_copy() in that order instead of the reverse order expected by rte_ether_addr_copy().

How can we fix this?

One way would be to introduce a new copy function and mark the old function deprecated (due to risk of bugs).
Does the community support such a change?
And what would be a good name for the new function?

Any other ideas for fixing it?


Med venlig hilsen / Kind regards,
-Morten Brørup

Re: Risk of rte_ether_addr_copy() causing bugs

[Stephen Hemminger]

On Mon, 4 Nov 2024 13:11:02 +0100
Morten Brørup <mb@smartsharesystems.com> wrote:

> Unlike memcpy() and other copy functions, rte_ether_addr_copy() takes the destination as the second parameter.
> 
> Not following well established conventions adds a high risk of causing bugs in the applications/libraries/drivers; it is likely that developers expect copy() functions to take parameters in the usual memcpy() order, and pass the parameters to rte_ether_addr_copy() in that order instead of the reverse order expected by rte_ether_addr_copy().
> 
> How can we fix this?
> 
> One way would be to introduce a new copy function and mark the old function deprecated (due to risk of bugs).
> Does the community support such a change?
> And what would be a good name for the new function?
> 
> Any other ideas for fixing it?
> 
> 
> Med venlig hilsen / Kind regards,
> -Morten Brørup
> 

Yes the order of arts is confusing.

For reference the Linux kernel has ether_addr_copy(dst, src)

Re: Risk of rte_ether_addr_copy() causing bugs

[Bruce Richardson]

On Mon, Nov 04, 2024 at 08:15:00AM -0800, Stephen Hemminger wrote:
> On Mon, 4 Nov 2024 13:11:02 +0100
> Morten Brørup <mb@smartsharesystems.com> wrote:
> 
> > Unlike memcpy() and other copy functions, rte_ether_addr_copy() takes the destination as the second parameter.
> > 
> > Not following well established conventions adds a high risk of causing bugs in the applications/libraries/drivers; it is likely that developers expect copy() functions to take parameters in the usual memcpy() order, and pass the parameters to rte_ether_addr_copy() in that order instead of the reverse order expected by rte_ether_addr_copy().
> > 
> > How can we fix this?
> > 
> > One way would be to introduce a new copy function and mark the old function deprecated (due to risk of bugs).
> > Does the community support such a change?
> > And what would be a good name for the new function?
> > 

Include "memcpy" in the name instead of copy, to make the order of args
clear? "rte_eth_addr_memcpy"?

Other thought is to do a macro version of the function, which would mean
that the name is capitalized.

/Bruce