Pointer type of _arp in __skb_flow_dissect()

2017-03-31 Thread Nicolas Iooss 
Hello,

Linux 4.11-rc4 contains the following code in function
__skb_flow_dissect(), file net/core/flow_dissector.c:

const struct arphdr *arp;
struct arphdr *_arp;

arp = __skb_header_pointer(skb, nhoff, sizeof(_arp), data,
   hlen, &_arp);


Here _arp and arp are both pointers to arphdr structures. In other calls
to __skb_header_pointer(), the buffer argument (_arp here) would have
been a struct instead of a pointer. What makes ARP packets different in
__skb_flow_dissect()?

Thanks,
Nicolas

PS: the code which I am curious about seems to have been introduced in
4.11-rc1 with commit 55733350e5e8 ("flow disector: ARP support")

qla3xxx: u32 constant overflow in fm93c56a_select()

2017-01-15 Thread Nicolas Iooss 
Hello,

In drivers/net/ethernet/qlogic/qla3xxx.c, fm93c56a_select() currently calls:

  ql_write_nvram_reg(qdev, spir,
((ISP_NVRAM_MASK << 16) | qdev->eeprom_cmd_data));

and ISP_NVRAM_MASK is defined as (0x000F << 16).

ql_write_nvram_reg() writes a 32-bit value but (ISP_NVRAM_MASK << 16)
expands to ((0x000F << 16) << 16) = 0xF << 32, which overflows the u32
type. This means the above call is equivalent to:

  ql_write_nvram_reg(qdev, spir, qdev->eeprom_cmd_data);

Is this something normal, in which case (ISP_NVRAM_MASK << 16) would be
removed, or a bug?

Thanks,
Nicolas

[PATCH 4/4] isdn/eicon: use const strings with format arguments

2016-10-29 Thread Nicolas Iooss 
Functions using a printf format argument do not modify the value of this
argument. These functions can therefore use type "const char *" instead
of "char *".

This patch has only been compile-tested.

Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
---
 drivers/isdn/hardware/eicon/debug.c| 14 +++---
 drivers/isdn/hardware/eicon/debuglib.h |  6 +++---
 drivers/isdn/hardware/eicon/maintidi.c |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/isdn/hardware/eicon/debug.c 
b/drivers/isdn/hardware/eicon/debug.c
index cd8d70e3292d..b8772bbee872 100644
--- a/drivers/isdn/hardware/eicon/debug.c
+++ b/drivers/isdn/hardware/eicon/debug.c
@@ -14,9 +14,9 @@
 
 static void DI_register(void *arg);
 static void DI_deregister(pDbgHandle hDbg);
-static void DI_format(int do_lock, word id, int type, char *format, va_list 
argument_list);
-static void DI_format_locked(word id, int type, char *format, va_list 
argument_list);
-static void DI_format_old(word id, char *format, va_list ap) { }
+static void DI_format(int do_lock, word id, int type, const char *format, 
va_list argument_list);
+static void DI_format_locked(word id, int type, const char *format, va_list 
argument_list);
+static void DI_format_old(word id, const char *format, va_list ap) { }
 static void DiProcessEventLog(unsigned short id, unsigned long msgID, va_list 
ap) { }
 static void single_p(byte *P, word *PLength, byte Id);
 static void diva_maint_xdi_cb(ENTITY *e);
@@ -25,7 +25,7 @@ static int diva_mnt_cmp_nmbr(const char *nmbr);
 static void diva_free_dma_descriptor(IDI_CALL request, int nr);
 static int diva_get_dma_descriptor(IDI_CALL request, dword *dma_magic);
 __printf(3, 4)
-void diva_mnt_internal_dprintf(dword drv_id, dword type, char *p, ...);
+void diva_mnt_internal_dprintf(dword drv_id, dword type, const char *p, ...);
 
 static dword MaxDumpSize = 256;
 static dword MaxXlogSize = 2 + 128;
@@ -561,7 +561,7 @@ static void DI_deregister(pDbgHandle hDbg) {
 
 static void DI_format_locked(unsigned short id,
 int type,
-char *format,
+const char *format,
 va_list argument_list) {
DI_format(1, id, type, format, argument_list);
 }
@@ -569,7 +569,7 @@ static void DI_format_locked(unsigned short id,
 static void DI_format(int do_lock,
  unsigned short id,
  int type,
- char *format,
+ const char *format,
  va_list ap) {
diva_os_spin_lock_magic_t old_irql;
dword sec, usec;
@@ -1904,7 +1904,7 @@ static void 
diva_change_management_debug_mask(diva_maint_client_t *pC, dword old
 }
 
 
-void diva_mnt_internal_dprintf(dword drv_id, dword type, char *fmt, ...) {
+void diva_mnt_internal_dprintf(dword drv_id, dword type, const char *fmt, ...) 
{
va_list ap;
 
va_start(ap, fmt);
diff --git a/drivers/isdn/hardware/eicon/debuglib.h 
b/drivers/isdn/hardware/eicon/debuglib.h
index 6dcbf6afb8f9..2170de140335 100644
--- a/drivers/isdn/hardware/eicon/debuglib.h
+++ b/drivers/isdn/hardware/eicon/debuglib.h
@@ -230,10 +230,10 @@ extern void DbgSetLevel(unsigned long dbgMask);
  */
 typedef struct _DbgHandle_ *pDbgHandle;
 typedef void (*DbgEnd)(pDbgHandle);
-typedef void (*DbgLog)(unsigned short, int, char *, va_list);
-typedef void (*DbgOld)(unsigned short, char *, va_list);
+typedef void (*DbgLog)(unsigned short, int, const char *, va_list);
+typedef void (*DbgOld)(unsigned short, const char *, va_list);
 typedef void (*DbgEv)(unsigned short, unsigned long, va_list);
-typedef void (*DbgIrq)(unsigned short, int, char *, va_list);
+typedef void (*DbgIrq)(unsigned short, int, const char *, va_list);
 typedef struct _DbgHandle_
 { charRegistered; /* driver successfully registered */
 #define DBG_HANDLE_REG_NEW 0x01  /* this (new) structure*/
diff --git a/drivers/isdn/hardware/eicon/maintidi.c 
b/drivers/isdn/hardware/eicon/maintidi.c
index b2ed2939b4fa..a635595e9be3 100644
--- a/drivers/isdn/hardware/eicon/maintidi.c
+++ b/drivers/isdn/hardware/eicon/maintidi.c
@@ -31,7 +31,7 @@
 
 
 extern __printf(3, 4)
-void diva_mnt_internal_dprintf(dword drv_id, dword type, char *p, ...);
+void diva_mnt_internal_dprintf(dword drv_id, dword type, const char *p, ...);
 
 #define MODEM_PARSE_ENTRIES  16 /* amount of variables of interest */
 #define FAX_PARSE_ENTRIES12 /* amount of variables of interest */
-- 
2.10.1

[PATCH 3/4] isdn/eicon: add some __printf attributes

2016-10-29 Thread Nicolas Iooss 
Add __printf attributes to some functions. This helps detecting errors
related to printf-formats at compile time.

When doing this, gcc reports some issues in debug.c. Fix them.

This patch has only been compile-tested.

Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
---
 drivers/isdn/hardware/eicon/debug.c| 129 +
 drivers/isdn/hardware/eicon/maintidi.c |   3 +-
 drivers/isdn/hardware/eicon/platform.h |   2 +-
 3 files changed, 68 insertions(+), 66 deletions(-)

diff --git a/drivers/isdn/hardware/eicon/debug.c 
b/drivers/isdn/hardware/eicon/debug.c
index 576b7b4a3278..cd8d70e3292d 100644
--- a/drivers/isdn/hardware/eicon/debug.c
+++ b/drivers/isdn/hardware/eicon/debug.c
@@ -24,6 +24,7 @@ static word SuperTraceCreateReadReq(byte *P, const char 
*path);
 static int diva_mnt_cmp_nmbr(const char *nmbr);
 static void diva_free_dma_descriptor(IDI_CALL request, int nr);
 static int diva_get_dma_descriptor(IDI_CALL request, dword *dma_magic);
+__printf(3, 4)
 void diva_mnt_internal_dprintf(dword drv_id, dword type, char *p, ...);
 
 static dword MaxDumpSize = 256;
@@ -1514,29 +1515,29 @@ static void diva_maint_state_change_notify(void 
*user_context,
}
 
 
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Ch= %lu",
+   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Ch= %u",
  (int)modem->ChannelNumber);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Event = %lu", modem->Event);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Norm  = %lu", modem->Norm);
+   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Event = %u", modem->Event);
+   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Norm  = %u", modem->Norm);
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Opts. = 0x%08x", modem->Options);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Tx= %lu Bps", modem->TxSpeed);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Rx= %lu Bps", modem->RxSpeed);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
RT= %lu mSec",
+   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Tx= %u Bps", modem->TxSpeed);
+   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Rx= %u Bps", modem->RxSpeed);
+   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
RT= %u mSec",
  modem->RoundtripMsec);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Sr= %lu", modem->SymbolRate);
+   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Sr= %u", modem->SymbolRate);
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
Rxl   = %d dBm", modem->RxLeveldBm);
diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
El= %d dBm", modem->EchoLeveldBm);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
SNR   = %lu dB", modem->SNRdb);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
MAE   = %lu", modem->MAE);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
LRet  = %lu",
+   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
SNR   = %u dB", modem->SNRdb);
+   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
MAE   = %u", modem->MAE);
+   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
LRet  = %u",
  modem->LocalRetrains);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
RRet  = %lu",
+   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
RRet  = %u",
  modem->RemoteRetrains);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
LRes  = %lu", modem->LocalResyncs);
-   diva_mnt_internal_dprintf(pC->hDbg->id, DLI_STAT, "MDM 
RRes  = %lu",
+   diva_mnt_internal_dprintf(pC->hDbg->id

[PATCH 2/4] isdn/eicon: fix some message formatting errors

2016-10-29 Thread Nicolas Iooss 
There are some inconsistent debug message formats in message.c. For
example,

dprintf("XDI CAPI: RC cancelled Id:0x02, Ch:%02x", e->Id, ch);

wrongly reports an ID of 2 and prints the entity ID as the channel ID.
There are also object pointers which are used instead of the IDs.

All these inconsistent formats have been found by adding __printf
attribute to myDbgPrint_...() functions (used by dbug()). As this makes
the compiler to also complain about using "%ld" with unsigned int values
(instead of "%u") and some other less-important format issues, this
patch does not add any __printf attribute.

This patch has only been compile-tested.

Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
---
 drivers/isdn/hardware/eicon/message.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/isdn/hardware/eicon/message.c 
b/drivers/isdn/hardware/eicon/message.c
index 1a1d99704fe6..7cafa34c3464 100644
--- a/drivers/isdn/hardware/eicon/message.c
+++ b/drivers/isdn/hardware/eicon/message.c
@@ -1059,7 +1059,7 @@ static void plci_remove(PLCI *plci)
}
if (plci->Sig.Id == 0xff)
{
-   dbug(1, dprintf("D-channel X.25 plci->NL.Id:%0x", plci->NL.Id));
+   dbug(1, dprintf("D-channel X.25 plci->NL.Id:%02x", 
plci->NL.Id));
if (plci->NL.Id && !plci->nl_remove_id)
{
nl_req_ncci(plci, REMOVE, 0);
@@ -3109,7 +3109,7 @@ static byte data_b3_req(dword Id, word Number, 
DIVA_CAPI_ADAPTER *a,
 
Info = _WRONG_IDENTIFIER;
ncci = (word)(Id >> 16);
-   dbug(1, dprintf("ncci=0x%x, plci=0x%x", ncci, plci));
+   dbug(1, dprintf("ncci=0x%x, plci=0x%x", ncci, plci->Id));
 
if (plci && ncci)
{
@@ -3325,7 +3325,7 @@ static byte select_b_req(dword Id, word Number, 
DIVA_CAPI_ADAPTER *a,
else
{
dbug(1, 
dprintf("select_b_req[%d],PLCI=0x%x,Tel=0x%x,NL=0x%x,appl=0x%x,sstate=0x%x",
-   msg->length, plci->Id, plci->tel, plci->NL.Id, 
plci->appl, plci->SuppState));
+   msg->length, plci->Id, plci->tel, plci->NL.Id, 
appl->Id, plci->SuppState));
dbug(1, dprintf("PlciState=0x%x", plci->State));
for (i = 0; i < 7; i++) bp_parms[i].length = 0;
 
@@ -3910,7 +3910,7 @@ void callback(ENTITY *e)
if (no_cancel_rc && (a->FlowControlIdTable[ch] 
== e->Id) && e->Id) {
a->FlowControlIdTable[ch] = 0;
if ((rc == OK) && 
a->FlowControlSkipTable[ch]) {
-   dbug(3, dprintf("XDI CAPI: RC 
cancelled Id:0x02, Ch:%02x", e->Id, ch));
+   dbug(3, dprintf("XDI CAPI: RC 
cancelled Id:%02x, Ch:%02x", e->Id, ch));
return;
}
}
@@ -9135,7 +9135,7 @@ static word AdvCodecSupport(DIVA_CAPI_ADAPTER *a, PLCI 
*plci, APPL *appl,
{
if (a->AdvSignalAppl != appl || a->AdvSignalPLCI)
{
-   dbug(1, dprintf("AdvSigPlci=0x%x", 
a->AdvSignalPLCI));
+   dbug(1, dprintf("AdvSigPlci=0x%x", 
a->AdvSignalPLCI->Id));
return 0x2001; /* codec in use by another 
application */
}
if (plci != NULL)
-- 
2.10.1

[PATCH 1/4] isdn/eicon: remove unused argument in DBG_ERR call

2016-10-29 Thread Nicolas Iooss 
diva_um_idi_read() can call DBG_ERR with 3 format arguments but using a
format string which only uses 2 of them. Remove the last one.

This bug has been found by adding a __printf attribute to
myDbgPrint_...() functions. As this addition leads the compiler to
report a lot of -Wformat warnings (for example the compiler complains
when "%08x" is used to format a pointer, as it is done with all usages
of "E(%08x)" in um_idi.c), this patch does not add any __printf
attribute.

This patch has only been compile-tested.

Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
---
 drivers/isdn/hardware/eicon/um_idi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/isdn/hardware/eicon/um_idi.c 
b/drivers/isdn/hardware/eicon/um_idi.c
index e1519718ce67..13ef38fa6cb0 100644
--- a/drivers/isdn/hardware/eicon/um_idi.c
+++ b/drivers/isdn/hardware/eicon/um_idi.c
@@ -351,7 +351,7 @@ int diva_um_idi_read(void *entity,
  Not enough space to read message
*/
DBG_ERR(("A: A(%d) E(%08x) read small buffer",
-a->adapter_nr, e, ret));
+a->adapter_nr, e));
diva_os_leave_spin_lock(_lock, _irql,
"read");
return (-2);
-- 
2.10.1

[PATCH 1/1] ath10k: use the right length of "background"

2016-10-29 Thread Nicolas Iooss 
The word "background" contains 10 characters so the third argument of
strncmp() need to be 10 in order to match this prefix correctly.

Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
Fixes: 855aed1220d2 ("ath10k: add spectral scan feature")
---
 drivers/net/wireless/ath/ath10k/spectral.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/spectral.c 
b/drivers/net/wireless/ath/ath10k/spectral.c
index 7d9b0da1b010..2ffc1fe4923b 100644
--- a/drivers/net/wireless/ath/ath10k/spectral.c
+++ b/drivers/net/wireless/ath/ath10k/spectral.c
@@ -338,7 +338,7 @@ static ssize_t write_file_spec_scan_ctl(struct file *file,
} else {
res = -EINVAL;
}
-   } else if (strncmp("background", buf, 9) == 0) {
+   } else if (strncmp("background", buf, 10) == 0) {
res = ath10k_spectral_scan_config(ar, SPECTRAL_BACKGROUND);
} else if (strncmp("manual", buf, 6) == 0) {
res = ath10k_spectral_scan_config(ar, SPECTRAL_MANUAL);
-- 
2.10.1

[PATCH v2 1/1] brcmfmac: fix pmksa->bssid usage

2016-08-23 Thread Nicolas Iooss 
The struct cfg80211_pmksa defines its bssid field as:

const u8 *bssid;

contrary to struct brcmf_pmksa, which uses:

u8 bssid[ETH_ALEN];

Therefore in brcmf_cfg80211_del_pmksa(), >bssid takes the address
of this field (of type u8**), not the one of its content (which would be
u8*).  Remove the & operator to make brcmf_dbg("%pM") and memcmp()
behave as expected.

This bug have been found using a custom static checker (which checks the
usage of %p... attributes at build time).  It has been introduced in
commit 6c404f34f2bd ("brcmfmac: Cleanup pmksa cache handling code"),
which replaced pmksa->bssid by >bssid while refactoring the code,
without modifying struct cfg80211_pmksa definition.

Replace [i].bssid with pmk[i].bssid too to make the code clearer,
this change does not affect the semantic.

Fixes: 6c404f34f2bd ("brcmfmac: Cleanup pmksa cache handling code")
Cc: sta...@vger.kernel.org
Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2628d5e12c64..201a98016142 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3884,11 +3884,11 @@ brcmf_cfg80211_del_pmksa(struct wiphy *wiphy, struct 
net_device *ndev,
if (!check_vif_up(ifp->vif))
return -EIO;
 
-   brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", >bssid);
+   brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", pmksa->bssid);
 
npmk = le32_to_cpu(cfg->pmk_list.npmk);
for (i = 0; i < npmk; i++)
-   if (!memcmp(>bssid, [i].bssid, ETH_ALEN))
+   if (!memcmp(pmksa->bssid, pmk[i].bssid, ETH_ALEN))
break;
 
if ((npmk > 0) && (i < npmk)) {
-- 
2.9.3

Re: [PATCH 1/1] brcmfmac: fix pmksa->bssid usage

2016-08-23 Thread Nicolas Iooss 
On 22/08/16 21:38, Arend Van Spriel wrote:
> On 22-8-2016 15:03, Nicolas Iooss wrote:
>> On 05/08/16 22:34, Nicolas Iooss wrote:
[...]
>>> Fixes: 6c404f34f2bd ("brcmfmac: Cleanup pmksa cache handling code")
>>> Cc: sta...@ger.kernel.org
> 
> Ah, so you did something wrong after all :-p. The email address should
> be 'sta...@vger.kernel.org'.

Thanks for spotting this! I'll fix this address.

>>> Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
>>> ---
>>>
>>> scripts/checkpatch.pl reports a warning: "Prefer ether_addr_equal() or
>>> ether_addr_equal_unaligned() over memcmp()".  Because some files in
>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/ still use memcmp()
>>> to compare addresses and because I do not know whether pmksa->bssid is
>>> always aligned, I did not follow this warning.
> 
> As most of this is done in slow path, I prefer memcmp() as I do not want
> to check alignment for minimal performance gain.

OK.

>>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> index 2628d5e12c64..aceab77cd95a 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> @@ -3884,11 +3884,11 @@ brcmf_cfg80211_del_pmksa(struct wiphy *wiphy, 
>>> struct net_device *ndev,
>>> if (!check_vif_up(ifp->vif))
>>> return -EIO;
>>>  
>>> -   brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", >bssid);
>>> +   brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", pmksa->bssid);
>>>  
>>> npmk = le32_to_cpu(cfg->pmk_list.npmk);
>>> for (i = 0; i < npmk; i++)
>>> -   if (!memcmp(>bssid, [i].bssid, ETH_ALEN))
>>> +   if (!memcmp(pmksa->bssid, [i].bssid, ETH_ALEN))
> 
> I find '[i].bssid' confusing so maybe you could change it to
> '[i].bssid[0]' or 'pmk[i].bssid' as I think these two are
> essentially the same.

I agree the three ways of writing this share the same meaning. I'll send
a v2 with 'pmk[i].bssid'.
> 
> Regards,
> Arend

Thanks for your review!
Nicolas

Re: [PATCH 1/1] brcmfmac: fix pmksa->bssid usage

2016-08-22 Thread Nicolas Iooss 
Hello,

After I sent the following patch a few weeks ago, I have not received
any feedback. Could you please review it and tell me what I may have
done wrong?

Thanks,
Nicolas

On 05/08/16 22:34, Nicolas Iooss wrote:
> The struct cfg80211_pmksa defines its bssid field as:
> 
> const u8 *bssid;
> 
> contrary to struct brcmf_pmksa, which uses:
> 
> u8 bssid[ETH_ALEN];
> 
> Therefore in brcmf_cfg80211_del_pmksa(), >bssid takes the address
> of this field (of type u8**), not the one of its content (which would be
> u8*).  Remove the & operator to make brcmf_dbg("%pM") and memcmp()
> behave as expected.
> 
> This bug have been found using a custom static checker (which checks the
> usage of %p... attributes at build time).  It has been introduced in
> commit 6c404f34f2bd ("brcmfmac: Cleanup pmksa cache handling code"),
> which replaced pmksa->bssid by >bssid while refactoring the code,
> without modifying struct cfg80211_pmksa definition.
> 
> Fixes: 6c404f34f2bd ("brcmfmac: Cleanup pmksa cache handling code")
> Cc: sta...@ger.kernel.org
> Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
> ---
> 
> scripts/checkpatch.pl reports a warning: "Prefer ether_addr_equal() or
> ether_addr_equal_unaligned() over memcmp()".  Because some files in
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/ still use memcmp()
> to compare addresses and because I do not know whether pmksa->bssid is
> always aligned, I did not follow this warning.
> 
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 2628d5e12c64..aceab77cd95a 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -3884,11 +3884,11 @@ brcmf_cfg80211_del_pmksa(struct wiphy *wiphy, struct 
> net_device *ndev,
>   if (!check_vif_up(ifp->vif))
>   return -EIO;
>  
> - brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", >bssid);
> + brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", pmksa->bssid);
>  
>   npmk = le32_to_cpu(cfg->pmk_list.npmk);
>   for (i = 0; i < npmk; i++)
> - if (!memcmp(>bssid, [i].bssid, ETH_ALEN))
> + if (!memcmp(pmksa->bssid, [i].bssid, ETH_ALEN))
>   break;
>  
>   if ((npmk > 0) && (i < npmk)) {
>

Re: [PATCH 1/1] Bluetooth: add printf format attribute to hci_set_[fh]w_info()

2016-08-15 Thread Nicolas Iooss 
Hello,
After I sent the following patch a few weeks ago, I have not received
any feedback. Could you please review it?

Thanks,
Nicolas

On 29/07/16 13:28, Nicolas Iooss wrote:
> Commit 5177a83827cd ("Bluetooth: Add debugfs fields for hardware and
> firmware info") introduced hci_set_hw_info() and hci_set_fw_info().
> These functions use kvasprintf_const() but are not marked with a
> __printf attribute.  Adding such an attribute helps detecting issues
> related to printf-formatting at build time.
> 
> Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
> ---
>  include/net/bluetooth/hci_core.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h 
> b/include/net/bluetooth/hci_core.h
> index ee7fc47680a1..012e5031fe47 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1026,8 +1026,8 @@ int hci_resume_dev(struct hci_dev *hdev);
>  int hci_reset_dev(struct hci_dev *hdev);
>  int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb);
>  int hci_recv_diag(struct hci_dev *hdev, struct sk_buff *skb);
> -void hci_set_hw_info(struct hci_dev *hdev, const char *fmt, ...);
> -void hci_set_fw_info(struct hci_dev *hdev, const char *fmt, ...);
> +__printf(2, 3) void hci_set_hw_info(struct hci_dev *hdev, const char *fmt, 
> ...);
> +__printf(2, 3) void hci_set_fw_info(struct hci_dev *hdev, const char *fmt, 
> ...);
>  int hci_dev_open(__u16 dev);
>  int hci_dev_close(__u16 dev);
>  int hci_dev_do_close(struct hci_dev *hdev);
>

[PATCH 1/1] brcmfmac: fix pmksa->bssid usage

2016-08-05 Thread Nicolas Iooss 
The struct cfg80211_pmksa defines its bssid field as:

const u8 *bssid;

contrary to struct brcmf_pmksa, which uses:

u8 bssid[ETH_ALEN];

Therefore in brcmf_cfg80211_del_pmksa(), >bssid takes the address
of this field (of type u8**), not the one of its content (which would be
u8*).  Remove the & operator to make brcmf_dbg("%pM") and memcmp()
behave as expected.

This bug have been found using a custom static checker (which checks the
usage of %p... attributes at build time).  It has been introduced in
commit 6c404f34f2bd ("brcmfmac: Cleanup pmksa cache handling code"),
which replaced pmksa->bssid by >bssid while refactoring the code,
without modifying struct cfg80211_pmksa definition.

Fixes: 6c404f34f2bd ("brcmfmac: Cleanup pmksa cache handling code")
Cc: sta...@ger.kernel.org
Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
---

scripts/checkpatch.pl reports a warning: "Prefer ether_addr_equal() or
ether_addr_equal_unaligned() over memcmp()".  Because some files in
drivers/net/wireless/broadcom/brcm80211/brcmfmac/ still use memcmp()
to compare addresses and because I do not know whether pmksa->bssid is
always aligned, I did not follow this warning.

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2628d5e12c64..aceab77cd95a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3884,11 +3884,11 @@ brcmf_cfg80211_del_pmksa(struct wiphy *wiphy, struct 
net_device *ndev,
if (!check_vif_up(ifp->vif))
return -EIO;
 
-   brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", >bssid);
+   brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", pmksa->bssid);
 
npmk = le32_to_cpu(cfg->pmk_list.npmk);
for (i = 0; i < npmk; i++)
-   if (!memcmp(>bssid, [i].bssid, ETH_ALEN))
+   if (!memcmp(pmksa->bssid, [i].bssid, ETH_ALEN))
break;
 
if ((npmk > 0) && (i < npmk)) {
-- 
2.9.2

[PATCH 1/1] RDS: add __printf format attribute to error reporting functions

2016-08-05 Thread Nicolas Iooss 
This is helpful to detect at compile-time errors related to format
strings.

Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
---
 net/rds/ib.h  | 1 +
 net/rds/rds.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/net/rds/ib.h b/net/rds/ib.h
index 046f7508c06b..45ac8e8e58f4 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -333,6 +333,7 @@ void rds_ib_conn_path_shutdown(struct rds_conn_path *cp);
 void rds_ib_state_change(struct sock *sk);
 int rds_ib_listen_init(void);
 void rds_ib_listen_stop(void);
+__printf(2, 3)
 void __rds_ib_conn_error(struct rds_connection *conn, const char *, ...);
 int rds_ib_cm_handle_connect(struct rdma_cm_id *cm_id,
 struct rdma_cm_event *event);
diff --git a/net/rds/rds.h b/net/rds/rds.h
index b2d17f0fafa8..fd0bccb2f9f9 100644
--- a/net/rds/rds.h
+++ b/net/rds/rds.h
@@ -688,6 +688,7 @@ void __rds_conn_error(struct rds_connection *conn, const 
char *, ...);
 #define rds_conn_error(conn, fmt...) \
__rds_conn_error(conn, KERN_WARNING "RDS: " fmt)
 
+__printf(2, 3)
 void __rds_conn_path_error(struct rds_conn_path *cp, const char *, ...);
 #define rds_conn_path_error(cp, fmt...) \
__rds_conn_path_error(cp, KERN_WARNING "RDS: " fmt)
-- 
2.9.2

[PATCH 1/1] Bluetooth: add printf format attribute to hci_set_[fh]w_info()

2016-07-29 Thread Nicolas Iooss 
Commit 5177a83827cd ("Bluetooth: Add debugfs fields for hardware and
firmware info") introduced hci_set_hw_info() and hci_set_fw_info().
These functions use kvasprintf_const() but are not marked with a
__printf attribute.  Adding such an attribute helps detecting issues
related to printf-formatting at build time.

Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
---
 include/net/bluetooth/hci_core.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ee7fc47680a1..012e5031fe47 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1026,8 +1026,8 @@ int hci_resume_dev(struct hci_dev *hdev);
 int hci_reset_dev(struct hci_dev *hdev);
 int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb);
 int hci_recv_diag(struct hci_dev *hdev, struct sk_buff *skb);
-void hci_set_hw_info(struct hci_dev *hdev, const char *fmt, ...);
-void hci_set_fw_info(struct hci_dev *hdev, const char *fmt, ...);
+__printf(2, 3) void hci_set_hw_info(struct hci_dev *hdev, const char *fmt, 
...);
+__printf(2, 3) void hci_set_fw_info(struct hci_dev *hdev, const char *fmt, 
...);
 int hci_dev_open(__u16 dev);
 int hci_dev_close(__u16 dev);
 int hci_dev_do_close(struct hci_dev *hdev);
-- 
2.9.0

[PATCH] iwlwifi: mvm: fix tof.h header guard

2015-09-12 Thread Nicolas Iooss 
Commit ce7929186a39 ("iwlwifi: mvm: add basic Time of Flight (802.11mc
FTM) support") created drivers/net/wireless/iwlwifi/mvm/tof.h with a
broken header guard:

#ifndef __tof
#define __tof_h__

...

#endif /* __tof_h__ */

Use __tof_h__ in the first line.

Signed-off-by: Nicolas Iooss <nicolas.iooss_li...@m4x.org>
---
 drivers/net/wireless/iwlwifi/mvm/tof.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/iwlwifi/mvm/tof.h 
b/drivers/net/wireless/iwlwifi/mvm/tof.h
index 50ae8adaaa6e..9beebc33cb8d 100644
--- a/drivers/net/wireless/iwlwifi/mvm/tof.h
+++ b/drivers/net/wireless/iwlwifi/mvm/tof.h
@@ -60,7 +60,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  *
  */
-#ifndef __tof
+#ifndef __tof_h__
 #define __tof_h__
 
 #include "fw-api-tof.h"
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html