Re: [PATCH] staging: slicoss: Fix prefer ether_addr_copy over memcpy
El 17/03/14 23:49, Jingoo Han escribió: On Tuesday, March 18, 2014 2:04 PM, Joel Pelaez Jorge wrote: @@ -810,8 +810,8 @@ static int slic_mac_set_address(struct net_device *dev, void *ptr) if (!is_valid_ether_addr(addr->sa_data)) return -EINVAL; - memcpy(dev->dev_addr, addr->sa_data, dev->addr_len); - memcpy(adapter->currmacaddr, addr->sa_data, dev->addr_len); + ether_addr_copy(dev->dev_addr, addr->sa_data); + ether_addr_copy(adapter->currmacaddr, addr->sa_data); By the way, I am wondering if 'dev->addr_len' is 6 bytes. Is there anyone who can confirm it? If nobody can confirm 'dev->addr_len' is 6 bytes, it should not be changed to 'ether_addr_copy()'. Best regards, Jingoo Han In the first case, is necessary use memcpy for copy te ethernet address to dev->dev_addr because this is a pointer, uses dev->addr_len for define size. But in the second, adapter->currmacaddr is fixed to 6 size, so that here one can use ether_addr_copy without issues. -- Best regards, Joel Pelaez Jorge Joel Pelaez Jorge -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: slicoss: Fix prefer ether_addr_copy over memcpy
On Mon, Mar 17, 2014 at 09:17:35PM -0600, Joel Pelaez Jorge wrote: > This patch fixes the following checkpatch.pl issues caused by the new > function: ether_addr_copy > > Signed-off-by: Joel Pelaez Jorge > --- > diff --git a/drivers/staging/slicoss/slicoss.c > b/drivers/staging/slicoss/slicoss.c > index 12aafe3..4ff39aa 100644 > --- a/drivers/staging/slicoss/slicoss.c > +++ b/drivers/staging/slicoss/slicoss.c > @@ -2313,7 +2313,7 @@ static int slic_mcast_add_list(struct adapter > *adapter, char *address) > if (mcaddr == NULL) > return 1; > > -memcpy(mcaddr->address, address, ETH_ALEN); > +ether_addr_copy(mcaddr->address, address); > > mcaddr->next = adapter->mcastaddrs; > adapter->mcastaddrs = mcaddr; The patch is line-wrapped, and tabs converted to spaces, making it impossible to apply the patch. Please fix your email client and try again. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: slicoss: Fix prefer ether_addr_copy over memcpy
On Mon, Mar 17, 2014 at 09:17:35PM -0600, Joel Pelaez Jorge wrote: This patch fixes the following checkpatch.pl issues caused by the new function: ether_addr_copy Signed-off-by: Joel Pelaez Jorge joelpel...@gmail.com --- diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c index 12aafe3..4ff39aa 100644 --- a/drivers/staging/slicoss/slicoss.c +++ b/drivers/staging/slicoss/slicoss.c @@ -2313,7 +2313,7 @@ static int slic_mcast_add_list(struct adapter *adapter, char *address) if (mcaddr == NULL) return 1; -memcpy(mcaddr-address, address, ETH_ALEN); +ether_addr_copy(mcaddr-address, address); mcaddr-next = adapter-mcastaddrs; adapter-mcastaddrs = mcaddr; The patch is line-wrapped, and tabs converted to spaces, making it impossible to apply the patch. Please fix your email client and try again. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: slicoss: Fix prefer ether_addr_copy over memcpy
El 17/03/14 23:49, Jingoo Han escribió: On Tuesday, March 18, 2014 2:04 PM, Joel Pelaez Jorge wrote: @@ -810,8 +810,8 @@ static int slic_mac_set_address(struct net_device *dev, void *ptr) if (!is_valid_ether_addr(addr-sa_data)) return -EINVAL; - memcpy(dev-dev_addr, addr-sa_data, dev-addr_len); - memcpy(adapter-currmacaddr, addr-sa_data, dev-addr_len); + ether_addr_copy(dev-dev_addr, addr-sa_data); + ether_addr_copy(adapter-currmacaddr, addr-sa_data); By the way, I am wondering if 'dev-addr_len' is 6 bytes. Is there anyone who can confirm it? If nobody can confirm 'dev-addr_len' is 6 bytes, it should not be changed to 'ether_addr_copy()'. Best regards, Jingoo Han In the first case, is necessary use memcpy for copy te ethernet address to dev-dev_addr because this is a pointer, uses dev-addr_len for define size. But in the second, adapter-currmacaddr is fixed to 6 size, so that here one can use ether_addr_copy without issues. -- Best regards, Joel Pelaez Jorge Joel Pelaez Jorge -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: slicoss: Fix prefer ether_addr_copy over memcpy
On Tuesday, March 18, 2014 2:04 PM, Joel Pelaez Jorge wrote: > > This patch fixes the following checkpatch.pl issues caused by the new > function: ether_addr_copy > > Signed-off-by: Joel Pelaez Jorge > --- > drivers/staging/slicoss/slicoss.c | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/staging/slicoss/slicoss.c > b/drivers/staging/slicoss/slicoss.c > index 12aafe3..0e0e374 100644 > --- a/drivers/staging/slicoss/slicoss.c > +++ b/drivers/staging/slicoss/slicoss.c > @@ -581,15 +581,15 @@ static void slic_adapter_set_hwaddr(struct adapter > *adapter) > struct sliccard *card = adapter->card; > > if ((adapter->card) && (card->config_set)) { > - memcpy(adapter->macaddr, > -card->config.MacInfo[adapter->functionnumber].macaddrA, > -sizeof(struct slic_config_mac)); > + ether_addr_copy(adapter->macaddr, > + card->config.MacInfo[adapter->functionnumber] > + .macaddrA); As declared in ./include/linux/etherdevice.h, Copied size should be a six-byte. static inline void ether_addr_copy(u8 *dst, const u8 *src) { #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) *(u32 *)dst = *(const u32 *)src; *(u16 *)(dst + 4) = *(const u16 *)(src + 4); #else u16 *a = (u16 *)dst; const u16 *b = (const u16 *)src; a[0] = b[0]; a[1] = b[1]; a[2] = b[2]; #endif } In this case, sizeof(struct slic_config_mac) will be 6 byte, as below. So, it looks good. ./drivers/staging/slicoss/slichw.h struct slic_config_mac { u8 macaddrA[6]; }; > if (is_zero_ether_addr(adapter->currmacaddr)) > - memcpy(adapter->currmacaddr, adapter->macaddr, > -ETH_ALEN); > + ether_addr_copy(adapter->currmacaddr, > + adapter->macaddr); > if (adapter->netdev) > - memcpy(adapter->netdev->dev_addr, adapter->currmacaddr, > -ETH_ALEN); > + ether_addr_copy(adapter->netdev->dev_addr, > + adapter->currmacaddr); > } > } ETH_ALEN is defined as 6. It looks good, too. ./include/uapi/linux/if_ether.h #define ETH_ALEN 6 > > @@ -810,8 +810,8 @@ static int slic_mac_set_address(struct net_device > *dev, void *ptr) > if (!is_valid_ether_addr(addr->sa_data)) > return -EINVAL; > > - memcpy(dev->dev_addr, addr->sa_data, dev->addr_len); > - memcpy(adapter->currmacaddr, addr->sa_data, dev->addr_len); > + ether_addr_copy(dev->dev_addr, addr->sa_data); > + ether_addr_copy(adapter->currmacaddr, addr->sa_data); By the way, I am wondering if 'dev->addr_len' is 6 bytes. Is there anyone who can confirm it? If nobody can confirm 'dev->addr_len' is 6 bytes, it should not be changed to 'ether_addr_copy()'. Best regards, Jingoo Han > > slic_config_set(adapter, true); > return 0; > @@ -2313,7 +2313,7 @@ static int slic_mcast_add_list(struct adapter > *adapter, char *address) > if (mcaddr == NULL) > return 1; > > - memcpy(mcaddr->address, address, ETH_ALEN); > + ether_addr_copy(mcaddr->address, address); > > mcaddr->next = adapter->mcastaddrs; > adapter->mcastaddrs = mcaddr; > -- > 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: slicoss: Fix prefer ether_addr_copy over memcpy
On Tue, 2014-03-18 at 12:38 +0900, Jingoo Han wrote: > In addition to this, there are the same cases like this, > in this file as below. Please search other same cases. > For instance, 'grep' will be a good way to find it. [] > static void slic_adapter_set_hwaddr(struct adapter *adapter) > { > struct sliccard *card = adapter->card; [] > if (is_zero_ether_addr(adapter->currmacaddr)) > memcpy(adapter->currmacaddr, adapter->macaddr, >ETH_ALEN); > if (adapter->netdev) > memcpy(adapter->netdev->dev_addr, > adapter->currmacaddr, >ETH_ALEN); > } > } > > Joe Perches, > These are not spotted by checkpatch.pl. > However, after modifying it as below, checkpatch warnings are > printed. Would you confirm it? If it's not on a single line, checkpatch won't find it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: slicoss: Fix prefer ether_addr_copy over memcpy
On Tuesday, March 18, 2014 12:18 PM, Joel Pelaez Jorge wrote: > > This patch fixes the following checkpatch.pl issues caused by the new > function: ether_addr_copy > > Signed-off-by: Joel Pelaez Jorge > --- > diff --git a/drivers/staging/slicoss/slicoss.c > b/drivers/staging/slicoss/slicoss.c > index 12aafe3..4ff39aa 100644 > --- a/drivers/staging/slicoss/slicoss.c > +++ b/drivers/staging/slicoss/slicoss.c > @@ -2313,7 +2313,7 @@ static int slic_mcast_add_list(struct adapter > *adapter, char *address) > if (mcaddr == NULL) > return 1; > > -memcpy(mcaddr->address, address, ETH_ALEN); > +ether_addr_copy(mcaddr->address, address); In addition to this, there are the same cases like this, in this file as below. Please search other same cases. For instance, 'grep' will be a good way to find it. static void slic_adapter_set_hwaddr(struct adapter *adapter) { struct sliccard *card = adapter->card; if ((adapter->card) && (card->config_set)) { memcpy(adapter->macaddr, card->config.MacInfo[adapter->functionnumber].macaddrA, sizeof(struct slic_config_mac)); if (is_zero_ether_addr(adapter->currmacaddr)) memcpy(adapter->currmacaddr, adapter->macaddr, ETH_ALEN); if (adapter->netdev) memcpy(adapter->netdev->dev_addr, adapter->currmacaddr, ETH_ALEN); } } Joe Perches, These are not spotted by checkpatch.pl. However, after modifying it as below, checkpatch warnings are printed. Would you confirm it? if (is_zero_ether_addr(adapter->currmacaddr)) memcpy(adapter->currmacaddr, adapter->macaddr, ETH_ALEN); if (adapter->netdev) memcpy(adapter->netdev->dev_addr, adapter->currmacaddr, ETH_ALEN); } Best regards, Jingoo Han > > mcaddr->next = adapter->mcastaddrs; > adapter->mcastaddrs = mcaddr; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: slicoss: Fix prefer ether_addr_copy over memcpy
On Tuesday, March 18, 2014 12:18 PM, Joel Pelaez Jorge wrote: This patch fixes the following checkpatch.pl issues caused by the new function: ether_addr_copy Signed-off-by: Joel Pelaez Jorge joelpel...@gmail.com --- diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c index 12aafe3..4ff39aa 100644 --- a/drivers/staging/slicoss/slicoss.c +++ b/drivers/staging/slicoss/slicoss.c @@ -2313,7 +2313,7 @@ static int slic_mcast_add_list(struct adapter *adapter, char *address) if (mcaddr == NULL) return 1; -memcpy(mcaddr-address, address, ETH_ALEN); +ether_addr_copy(mcaddr-address, address); In addition to this, there are the same cases like this, in this file as below. Please search other same cases. For instance, 'grep' will be a good way to find it. static void slic_adapter_set_hwaddr(struct adapter *adapter) { struct sliccard *card = adapter-card; if ((adapter-card) (card-config_set)) { memcpy(adapter-macaddr, card-config.MacInfo[adapter-functionnumber].macaddrA, sizeof(struct slic_config_mac)); if (is_zero_ether_addr(adapter-currmacaddr)) memcpy(adapter-currmacaddr, adapter-macaddr, ETH_ALEN); if (adapter-netdev) memcpy(adapter-netdev-dev_addr, adapter-currmacaddr, ETH_ALEN); } } Joe Perches, These are not spotted by checkpatch.pl. However, after modifying it as below, checkpatch warnings are printed. Would you confirm it? if (is_zero_ether_addr(adapter-currmacaddr)) memcpy(adapter-currmacaddr, adapter-macaddr, ETH_ALEN); if (adapter-netdev) memcpy(adapter-netdev-dev_addr, adapter-currmacaddr, ETH_ALEN); } Best regards, Jingoo Han mcaddr-next = adapter-mcastaddrs; adapter-mcastaddrs = mcaddr; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: slicoss: Fix prefer ether_addr_copy over memcpy
On Tue, 2014-03-18 at 12:38 +0900, Jingoo Han wrote: In addition to this, there are the same cases like this, in this file as below. Please search other same cases. For instance, 'grep' will be a good way to find it. [] static void slic_adapter_set_hwaddr(struct adapter *adapter) { struct sliccard *card = adapter-card; [] if (is_zero_ether_addr(adapter-currmacaddr)) memcpy(adapter-currmacaddr, adapter-macaddr, ETH_ALEN); if (adapter-netdev) memcpy(adapter-netdev-dev_addr, adapter-currmacaddr, ETH_ALEN); } } Joe Perches, These are not spotted by checkpatch.pl. However, after modifying it as below, checkpatch warnings are printed. Would you confirm it? If it's not on a single line, checkpatch won't find it. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: slicoss: Fix prefer ether_addr_copy over memcpy
On Tuesday, March 18, 2014 2:04 PM, Joel Pelaez Jorge wrote: This patch fixes the following checkpatch.pl issues caused by the new function: ether_addr_copy Signed-off-by: Joel Pelaez Jorge joelpel...@gmail.com --- drivers/staging/slicoss/slicoss.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c index 12aafe3..0e0e374 100644 --- a/drivers/staging/slicoss/slicoss.c +++ b/drivers/staging/slicoss/slicoss.c @@ -581,15 +581,15 @@ static void slic_adapter_set_hwaddr(struct adapter *adapter) struct sliccard *card = adapter-card; if ((adapter-card) (card-config_set)) { - memcpy(adapter-macaddr, -card-config.MacInfo[adapter-functionnumber].macaddrA, -sizeof(struct slic_config_mac)); + ether_addr_copy(adapter-macaddr, + card-config.MacInfo[adapter-functionnumber] + .macaddrA); As declared in ./include/linux/etherdevice.h, Copied size should be a six-byte. static inline void ether_addr_copy(u8 *dst, const u8 *src) { #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) *(u32 *)dst = *(const u32 *)src; *(u16 *)(dst + 4) = *(const u16 *)(src + 4); #else u16 *a = (u16 *)dst; const u16 *b = (const u16 *)src; a[0] = b[0]; a[1] = b[1]; a[2] = b[2]; #endif } In this case, sizeof(struct slic_config_mac) will be 6 byte, as below. So, it looks good. ./drivers/staging/slicoss/slichw.h struct slic_config_mac { u8 macaddrA[6]; }; if (is_zero_ether_addr(adapter-currmacaddr)) - memcpy(adapter-currmacaddr, adapter-macaddr, -ETH_ALEN); + ether_addr_copy(adapter-currmacaddr, + adapter-macaddr); if (adapter-netdev) - memcpy(adapter-netdev-dev_addr, adapter-currmacaddr, -ETH_ALEN); + ether_addr_copy(adapter-netdev-dev_addr, + adapter-currmacaddr); } } ETH_ALEN is defined as 6. It looks good, too. ./include/uapi/linux/if_ether.h #define ETH_ALEN 6 @@ -810,8 +810,8 @@ static int slic_mac_set_address(struct net_device *dev, void *ptr) if (!is_valid_ether_addr(addr-sa_data)) return -EINVAL; - memcpy(dev-dev_addr, addr-sa_data, dev-addr_len); - memcpy(adapter-currmacaddr, addr-sa_data, dev-addr_len); + ether_addr_copy(dev-dev_addr, addr-sa_data); + ether_addr_copy(adapter-currmacaddr, addr-sa_data); By the way, I am wondering if 'dev-addr_len' is 6 bytes. Is there anyone who can confirm it? If nobody can confirm 'dev-addr_len' is 6 bytes, it should not be changed to 'ether_addr_copy()'. Best regards, Jingoo Han slic_config_set(adapter, true); return 0; @@ -2313,7 +2313,7 @@ static int slic_mcast_add_list(struct adapter *adapter, char *address) if (mcaddr == NULL) return 1; - memcpy(mcaddr-address, address, ETH_ALEN); + ether_addr_copy(mcaddr-address, address); mcaddr-next = adapter-mcastaddrs; adapter-mcastaddrs = mcaddr; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/