Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
Hi Dan, On Sat, Oct 20, 2018 at 9:22 PM Dan Carpenter wrote: > > On Sat, Oct 20, 2018 at 04:42:07PM +0200, Miguel Ojeda wrote: > > Using an attribute is indeed better whenever possible. In C++17 it is > > an standard attribute and there have been proposals to include some of > > them for C as well since 2016 at least, e.g. the latest for > > fallthrough at: > > > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2268.pdf > > > > I have taken a quick look into supporting it (typing it here to save > > it on the mailing list :-), and we have: > > > > * gcc >= 7.1 supports -Wimplicit-fallthrough with > > __attribute__((fallthrough)), the comment syntax and the C++ > > [[fallthrough]] syntax. > > * gcc < 7.1 complains about empty declarations (it does not know > > about attributes for null statements) and also > > -Wdeclaration-after-statement. > > I'm not sure I understand about empty decalarations. The idea is that > we would do: That paragraph tried to explain that gcc < 7.1 did not know about __attribute__((fallthrough)); i.e. that everything was introduced in gcc 7.1. Anyway, the conclusion was a neuron misfiring of mine -- in my mind I was thinking clang supported the comment syntax (when I clearly wrote that it didn't). Never mind --- thanks for pointing it out! > > case 3: > frob(); > __fall_through(); > case 4: > frob(); > > #if GCC > 7.1 > #define __fall_through() __attribute__((fallthrough)) > #else > #define __fall_through() > #endif Yes, of course, that is what we do for other attributes -- actually in -next we have pending a better way for checking, using __has_attribute: #if __has_attribute(fallthrough) #define __fallthrough __attribute__((fallthrough)) #else #define __fallthrough #endif > > So long as GCC and static analysis tools understand about the attribute > that's enought to catch the bugs. No one cares what clang and icc do. Not so sure about that -- there are quite some people looking forward to building Linux with clang, even if only to have another compiler to check for warnings and to use the clang/llvm-related tools (and to write new ones). > We would just disable the fall through warnings on those until they are > updated to support the fallthrough attribute. You mean if they start supporting the warning but not the attribute? I don't think that would be likely (i.e. if clang enables the warning on C mode, they will have to introduce a way to suppress it; which should be the attribute (at least), since they typically like to be compatible with gcc and since they already support it in C++ >= 11), but everything is possible. > > We wouldn't update all the fall through comments until later, but going > forward people could use the __fall_through() macro if they want. Agreed. I will introduce it in the compiler-attributes tree -- should be there for -rc1 if no one complains. CC'ing all of you in the patch. Cheers, Miguel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
On Sat, Oct 20, 2018 at 04:42:07PM +0200, Miguel Ojeda wrote: > +On Wed, Oct 17, 2018 at 8:25 AM Dan Carpenter > wrote: > > > > It's not common at all. It should be wrapped in a macro and put into > > compiler.h. > > > > But I hope it does become adopted. It's better than randomly grepping > > for non-standard comments. > > Using an attribute is indeed better whenever possible. In C++17 it is > an standard attribute and there have been proposals to include some of > them for C as well since 2016 at least, e.g. the latest for > fallthrough at: > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2268.pdf > > I have taken a quick look into supporting it (typing it here to save > it on the mailing list :-), and we have: > > * gcc >= 7.1 supports -Wimplicit-fallthrough with > __attribute__((fallthrough)), the comment syntax and the C++ > [[fallthrough]] syntax. > * gcc < 7.1 complains about empty declarations (it does not know > about attributes for null statements) and also > -Wdeclaration-after-statement. I'm not sure I understand about empty decalarations. The idea is that we would do: case 3: frob(); __fall_through(); case 4: frob(); #if GCC > 7.1 #define __fall_through() __attribute__((fallthrough)) #else #define __fall_through() #endif So long as GCC and static analysis tools understand about the attribute that's enought to catch the bugs. No one cares what clang and icc do. We would just disable the fall through warnings on those until they are updated to support the fallthrough attribute. We wouldn't update all the fall through comments until later, but going forward people could use the __fall_through() macro if they want. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Solutions for you
We are one image studio who is able to process 300+ photos a day. If you need any image editing, please let us know. We can do it for you such as: Image cut out for photos and clipping path, masking for your photos, They are mostly used for ecommerce photos, jewelry photos retouching, beauty and skin images and wedding photos. We do also different kind of beauty retouching, portraits retouching. We can send editing for your photos if you send us one or two photos. Thanks, Linda ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
+On Wed, Oct 17, 2018 at 8:25 AM Dan Carpenter wrote: > > It's not common at all. It should be wrapped in a macro and put into > compiler.h. > > But I hope it does become adopted. It's better than randomly grepping > for non-standard comments. Using an attribute is indeed better whenever possible. In C++17 it is an standard attribute and there have been proposals to include some of them for C as well since 2016 at least, e.g. the latest for fallthrough at: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2268.pdf I have taken a quick look into supporting it (typing it here to save it on the mailing list :-), and we have: * gcc >= 7.1 supports -Wimplicit-fallthrough with __attribute__((fallthrough)), the comment syntax and the C++ [[fallthrough]] syntax. * gcc < 7.1 complains about empty declarations (it does not know about attributes for null statements) and also -Wdeclaration-after-statement. * clang 7 supports -Wimplicit-fallthrough (not enabled in -Wall/extra/pedantic like gcc, though) but *only* in C++ mode and with the C++ syntax [[fallthrough]]. In other words, in C mode, no syntax works and no diagnostics are emitted. It complains about Wmissing-declarations. [IMO they should allow the __attribute__ syntax for fallthrough (and enable it on C mode) to be compatible with gcc. Maybe they are simply waiting for the C2x attributes... :-)] * icc 19 does not know about -Wimplicit-fallthrough at all (but seems to allow [[fallthrough]] on C++17 mode to comply with the standard). Therefore, the only improvement we could do right now is starting to use the attribute for gcc > 7.1, and a comment for everybody else. However, even if that was worth the trouble of changing the 2500+ instances of fall through markings that we have, comments are replaced before the preprocessor stage, so we would need some (probably non-portable) macro magic. So, I would say, let's revisit this again in a few years. Possibly, when we move the minimum version to gcc 7.1, clang and icc may both support the fallthrough warning in C mode; so that we can always use the __attribute__((fallthrough)) unconditionally under a __fallthrough #define. Cheers, Miguel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: mt7621-dma: fix potentially dereferencing uninitialized 'tx_desc'
Function 'mtk_hsdma_start_transfer' uses 'tx_desc' pointer which can be
dereferenced before it is initializated. Initializate pointer before
avoiding the problem.
Fixes: 0853c7a53eb3: "staging: mt7621-dma: ralink: add rt2880 dma
engine"
Reported-by: Dan Carpenter
Signed-off-by: Sergio Paracuellos
---
drivers/staging/mt7621-dma/mtk-hsdma.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/mt7621-dma/mtk-hsdma.c
b/drivers/staging/mt7621-dma/mtk-hsdma.c
index df6ebf4..5831f81 100644
--- a/drivers/staging/mt7621-dma/mtk-hsdma.c
+++ b/drivers/staging/mt7621-dma/mtk-hsdma.c
@@ -335,6 +335,8 @@ static int mtk_hsdma_start_transfer(struct mtk_hsdam_engine
*hsdma,
/* tx desc */
src = sg->src_addr;
for (i = 0; i < chan->desc->num_sgs; i++) {
+ tx_desc = >tx_ring[chan->tx_idx];
+
if (len > HSDMA_MAX_PLEN)
tlen = HSDMA_MAX_PLEN;
else
@@ -344,7 +346,6 @@ static int mtk_hsdma_start_transfer(struct mtk_hsdam_engine
*hsdma,
tx_desc->addr1 = src;
tx_desc->flags |= HSDMA_DESC_PLEN1(tlen);
} else {
- tx_desc = >tx_ring[chan->tx_idx];
tx_desc->addr0 = src;
tx_desc->flags = HSDMA_DESC_PLEN0(tlen);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: rtl8188eu: simplify odm_evm_db_to_percentage()
Use clamp() to simplify code in odm_evm_db_to_percentage().
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/hal/odm_hwconfig.c | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
b/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
index 82d6b2e18b29..0c034b19d3dd 100644
--- a/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
+++ b/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
@@ -53,20 +53,11 @@ static s32 odm_signal_scale_mapping(struct odm_dm_struct
*dm_odm, s32 currsig)
static u8 odm_evm_db_to_percentage(s8 value)
{
/* -33dB~0dB to 0%~99% */
- s8 ret_val;
-
- ret_val = value;
-
- if (ret_val >= 0)
- ret_val = 0;
- if (ret_val <= -33)
- ret_val = -33;
-
- ret_val = 0 - ret_val;
- ret_val *= 3;
+ s8 ret_val = clamp(-value, 0, 33) * 3;
if (ret_val == 99)
ret_val = 100;
+
return ret_val;
}
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: rtl8188eu: rename variable Max_spatial_stream - style
Rename the variable Max_spatial_stream to avoid CamelCase.
Max_spatial_stream -> max_spatial_stream
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/hal/odm_hwconfig.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
b/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
index 0c034b19d3dd..d7bdbb141b2b 100644
--- a/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
+++ b/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
@@ -67,7 +67,7 @@ static void odm_RxPhyStatus92CSeries_Parsing(struct
odm_dm_struct *dm_odm,
struct odm_per_pkt_info *pPktinfo)
{
struct sw_ant_switch *pDM_SWAT_Table = _odm->DM_SWAT_Table;
- u8 i, Max_spatial_stream;
+ u8 i, max_spatial_stream;
s8 rx_pwr[4], rx_pwr_all = 0;
u8 EVM, PWDB_ALL = 0, PWDB_ALL_BT;
u8 RSSI, total_rssi = 0;
@@ -215,11 +215,11 @@ static void odm_RxPhyStatus92CSeries_Parsing(struct
odm_dm_struct *dm_odm,
/* (3)EVM of HT rate */
if (pPktinfo->Rate >= DESC92C_RATEMCS8 && pPktinfo->Rate <=
DESC92C_RATEMCS15)
- Max_spatial_stream = 2; /* both spatial stream make
sense */
+ max_spatial_stream = 2; /* both spatial stream make
sense */
else
- Max_spatial_stream = 1; /* only spatial stream 1 makes
sense */
+ max_spatial_stream = 1; /* only spatial stream 1 makes
sense */
- for (i = 0; i < Max_spatial_stream; i++) {
+ for (i = 0; i < max_spatial_stream; i++) {
/* Do not use shift operation like "rx_evmX >>= 1"
because the compilor of free build environment */
/* fill most significant bit to "zero" when doing
shifting operation which may change a negative */
/* value to positive one, then the dbm value (which is
supposed to be negative) is not correct anymore. */
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: rtl8188eu: rename variable isCCKrate - style
Rename the variable isCCKrate to avoid CamelCase.
isCCKrate -> is_cck_rate
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/hal/odm_hwconfig.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
b/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
index d7bdbb141b2b..5d72d7f6d05f 100644
--- a/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
+++ b/drivers/staging/rtl8188eu/hal/odm_hwconfig.c
@@ -71,19 +71,20 @@ static void odm_RxPhyStatus92CSeries_Parsing(struct
odm_dm_struct *dm_odm,
s8 rx_pwr[4], rx_pwr_all = 0;
u8 EVM, PWDB_ALL = 0, PWDB_ALL_BT;
u8 RSSI, total_rssi = 0;
- u8 isCCKrate = 0;
+ u8 is_cck_rate = 0;
u8 rf_rx_num = 0;
u8 cck_highpwr = 0;
u8 LNA_idx, VGA_idx;
struct phy_status_rpt *pPhyStaRpt = (struct phy_status_rpt *)pPhyStatus;
- isCCKrate = ((pPktinfo->Rate >= DESC92C_RATE1M) && (pPktinfo->Rate <=
DESC92C_RATE11M)) ? true : false;
+ is_cck_rate = ((pPktinfo->Rate >= DESC92C_RATE1M) &&
+ (pPktinfo->Rate <= DESC92C_RATE11M)) ? true : false;
pPhyInfo->RxMIMOSignalQuality[RF_PATH_A] = -1;
pPhyInfo->RxMIMOSignalQuality[RF_PATH_B] = -1;
- if (isCCKrate) {
+ if (is_cck_rate) {
u8 cck_agc_rpt;
dm_odm->PhyDbgInfo.NumQryPhyStatusCCK++;
@@ -234,7 +235,7 @@ static void odm_RxPhyStatus92CSeries_Parsing(struct
odm_dm_struct *dm_odm,
}
/* UI BSS List signal strength(in percentage), make it good looking,
from 0~100. */
/* It is assigned to the BSS List in GetValueFromBeaconOrProbeRsp(). */
- if (isCCKrate) {
+ if (is_cck_rate) {
pPhyInfo->SignalStrength =
(u8)(odm_signal_scale_mapping(dm_odm, PWDB_ALL));/* PWDB_ALL; */
} else {
if (rf_rx_num != 0)
@@ -255,7 +256,7 @@ static void odm_Process_RSSIForDM(struct odm_dm_struct
*dm_odm,
{
s32 UndecoratedSmoothedPWDB, UndecoratedSmoothedCCK;
s32 UndecoratedSmoothedOFDM, RSSI_Ave;
- u8 isCCKrate = 0;
+ u8 is_cck_rate = 0;
u8 RSSI_max, RSSI_min, i;
u32 OFDM_pkt = 0;
u32 Weighting = 0;
@@ -271,7 +272,8 @@ static void odm_Process_RSSIForDM(struct odm_dm_struct
*dm_odm,
if ((!pPktinfo->bPacketMatchBSSID))
return;
- isCCKrate = ((pPktinfo->Rate >= DESC92C_RATE1M) && (pPktinfo->Rate <=
DESC92C_RATE11M)) ? true : false;
+ is_cck_rate = ((pPktinfo->Rate >= DESC92C_RATE1M) &&
+ (pPktinfo->Rate <= DESC92C_RATE11M)) ? true : false;
/* Smart Antenna Debug Message-- */
@@ -299,7 +301,7 @@ static void odm_Process_RSSIForDM(struct odm_dm_struct
*dm_odm,
UndecoratedSmoothedPWDB = pEntry->rssi_stat.UndecoratedSmoothedPWDB;
if (pPktinfo->bPacketToSelf || pPktinfo->bPacketBeacon) {
- if (!isCCKrate) { /* ofdm rate */
+ if (!is_cck_rate) { /* ofdm rate */
if (pPhyInfo->RxMIMOSignalStrength[RF_PATH_B] == 0) {
RSSI_Ave =
pPhyInfo->RxMIMOSignalStrength[RF_PATH_A];
} else {
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
