On Fri, Jul 30, 2021 at 08:14:52PM +0200, Fabio M. De Francesco wrote:
> Fix sparse warnings of different base types in assignments
> and in passing function parameters.
This patch fixes some endian bugs but it's not mentioned at all in the
commit message. Did you send to the correct patch?
>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@xxxxxxxxx>
> ---
> drivers/staging/r8188eu/core/rtw_br_ext.c | 46 ++++++++++++++++++-----
> 1 file changed, 36 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c
> index e00302137a60..31ca2e548555 100644
> --- a/drivers/staging/r8188eu/core/rtw_br_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c
> @@ -71,7 +71,7 @@ static inline int __nat25_add_pppoe_tag(struct sk_buff *skb, struct pppoe_tag *t
> struct pppoe_hdr *ph = (struct pppoe_hdr *)(skb->data + ETH_HLEN);
> int data_len;
>
> - data_len = tag->tag_len + TAG_HDR_LEN;
> + data_len = be16_to_cpu(tag->tag_len) + TAG_HDR_LEN;
> if (skb_tailroom(skb) < data_len) {
> _DEBUG_ERR("skb_tailroom() failed in add SID tag!\n");
> return -1;
> @@ -134,42 +134,68 @@ static inline void __nat25_generate_ipv4_network_addr(unsigned char *networkAddr
> }
>
> static inline void __nat25_generate_ipx_network_addr_with_node(unsigned char *networkAddr,
> - unsigned int *ipxNetAddr, unsigned char *ipxNodeAddr)
> + __be32 *ipxNetAddr, unsigned char *ipxNodeAddr)
> {
> + union {
> + unsigned int f0;
> + unsigned char f1[IPX_NODE_LEN];
What is going on here?? Why is f1 six bytes?
> + } addr;
> +
> memset(networkAddr, 0, MAX_NETWORK_ADDR_LEN);
>
> networkAddr[0] = NAT25_IPX;
> - memcpy(networkAddr+1, (unsigned char *)ipxNetAddr, 4);
> + addr.f0 = be32_to_cpu(*ipxNetAddr);
> + memcpy(networkAddr+1, addr.f1, 4);
What's the point of a union? memcpy() doesn't care about endian
anotations.
> memcpy(networkAddr+5, ipxNodeAddr, 6);
> }
>
> static inline void __nat25_generate_ipx_network_addr_with_socket(unsigned char *networkAddr,
> - unsigned int *ipxNetAddr, unsigned short *ipxSocketAddr)
> + __be32 *ipxNetAddr, __be16 *ipxSocketAddr)
> {
> + union {
> + unsigned int f0;
> + unsigned char f1[4];
> + } addr;
> +
> memset(networkAddr, 0, MAX_NETWORK_ADDR_LEN);
>
> networkAddr[0] = NAT25_IPX;
> - memcpy(networkAddr+1, (unsigned char *)ipxNetAddr, 4);
> - memcpy(networkAddr+5, (unsigned char *)ipxSocketAddr, 2);
> + addr.f0 = be32_to_cpu(*ipxNetAddr);
> + memcpy(networkAddr+1, addr.f1, 4);
> + addr.f0 ^= addr.f0;
What on earth????
> + addr.f0 = be16_to_cpu(*ipxSocketAddr);
I'm so puzzled.
> + memcpy(networkAddr+5, addr.f1, 2);
This patch is really weird so I'm done reviewing it.
regards,
dan carpenter
