[PATCH RESEND 1/4] HID: multitouch: add MT_QUIRK_NOT_SEEN_MEANS_UP to MT_CLS_WIN_8 quirks
The firmware found in the touch screen of the Surface Pro 3 is slightly
buggy and occasionally doesn't send lift off reports for contacts; add
MT_QUIRK_NOT_SEEN_MEANS_UP to .quirks to compensate for the missed
reports.
Signed-off-by: Joey Pabalinas
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 45968f7970f87775fa..a793076139d7d0db9b 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -242,11 +242,12 @@ static struct mt_class mt_classes[] = {
.quirks = MT_QUIRK_ALWAYS_VALID |
MT_QUIRK_IGNORE_DUPLICATES |
MT_QUIRK_HOVERING |
MT_QUIRK_CONTACT_CNT_ACCURATE |
MT_QUIRK_STICKY_FINGERS |
- MT_QUIRK_WIN8_PTP_BUTTONS },
+ MT_QUIRK_WIN8_PTP_BUTTONS |
+ MT_QUIRK_NOT_SEEN_MEANS_UP },
{ .name = MT_CLS_EXPORT_ALL_INPUTS,
.quirks = MT_QUIRK_ALWAYS_VALID |
MT_QUIRK_CONTACT_CNT_ACCURATE,
.export_all_inputs = true },
{ .name = MT_CLS_WIN_8_DUAL,
--
2.18.0
[PATCH RESEND 3/4] HID: multitouch: drop reports containing invalid values
Avoid processing reports containing invalid values to reduce
multitouch input stutter.
Signed-off-by: Joey Pabalinas
1 file changed, 9 insertions(+)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index c0654db0b736543ca0..08b50e5908cecdda66 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -694,10 +694,19 @@ static void mt_complete_slot(struct mt_device *td, struct
input_dev *input)
{
if ((td->mtclass.quirks & MT_QUIRK_CONTACT_CNT_ACCURATE) &&
td->num_received >= td->num_expected)
return;
+ /* drop invalid values after counting them */
+ if (td->curdata.x == 0x &&
+ td->curdata.y == 0x &&
+ td->curdata.w == 0x &&
+ td->curdata.h == 0x) {
+ td->num_received++;
+ return;
+ }
+
if (td->curvalid || (td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID)) {
int active;
int slotnum = mt_compute_slot(td, input);
struct mt_slot *s = &td->curdata;
struct input_mt *mt = input->mt;
--
2.18.0
[PATCH RESEND 0/4] reduce Surface Pro 3 multitouch jitter
The Surface Pro 3 firmware doesn't reliably send contact lift off reports nor handle invalid report values gracefully. To reduce touchscreen input jitter: - add MT_QUIRK_NOT_SEEN_MEANS_UP to the MT_CLS_WIN_8 - drop invalid report values Patches have been tested on my personal Surface Pro 3 for a couple months without any problems, as well as being run in my Arch Linux AUR kernel package [1] without a single complaint so far. [1] https://aur.archlinux.org/packages/linux-surfacepro3-git Joey Pabalinas (4): HID: multitouch: add MT_QUIRK_NOT_SEEN_MEANS_UP to MT_CLS_WIN_8 quirks HID: multitouch: don't check HID_GROUP_MULTITOUCH_WIN_8 for serial protocol HID: multitouch: drop reports containing invalid values HID: multitouch: remove unneeded else conditional cases drivers/hid/hid-multitouch.c | 27 --- 1 file changed, 20 insertions(+), 7 deletions(-) -- 2.18.0
[PATCH RESEND 2/4] HID: multitouch: don't check HID_GROUP_MULTITOUCH_WIN_8 for serial protocol
The HID_GROUP_MULTITOUCH_WIN_8 group never needs to check for the serial
protocol, so avoid setting `td->serial_maybe = true;` in order to avoid
an unnecessary mt_post_parse_default_settings() call
Signed-off-by: Joey Pabalinas
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index a793076139d7d0db9b..c0654db0b736543ca0 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1460,11 +1460,13 @@ static int mt_probe(struct hid_device *hdev, const
struct hid_device_id *id)
if (!td->fields) {
dev_err(&hdev->dev, "cannot allocate multitouch fields data\n");
return -ENOMEM;
}
- if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
+ if (id->vendor == HID_ANY_ID
+ && id->product == HID_ANY_ID
+ && id->group != HID_GROUP_MULTITOUCH_WIN_8)
td->serial_maybe = true;
/* This allows the driver to correctly support devices
* that emit events over several HID messages.
*/
--
2.18.0
[PATCH RESEND 4/4] HID: multitouch: remove unneeded else conditional cases
Elide lone `else` cases and replace `else if` clauses
with plain `if` conditionals when they occur immediately
after return statements.
Signed-off-by: Joey Pabalinas
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 08b50e5908cecdda66..30b1a2c67f39a41325 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -205,12 +205,12 @@ static void mt_post_parse(struct mt_device *td);
static int cypress_compute_slot(struct mt_device *td)
{
if (td->curdata.contactid != 0 || td->num_received == 0)
return td->curdata.contactid;
- else
- return -1;
+
+ return -1;
}
static struct mt_class mt_classes[] = {
{ .name = MT_CLS_DEFAULT,
.quirks = MT_QUIRK_ALWAYS_VALID |
@@ -803,12 +803,12 @@ static int mt_compute_timestamp(struct mt_device *td,
struct hid_field *field,
delta *= 100;
if (jdelta > MAX_TIMESTAMP_INTERVAL)
/* No data received for a while, resync the timestamp. */
return 0;
- else
- return td->timestamp + delta;
+
+ return td->timestamp + delta;
}
static int mt_touch_event(struct hid_device *hid, struct hid_field *field,
struct hid_usage *usage, __s32 value)
{
@@ -1110,11 +1110,12 @@ static int mt_input_mapping(struct hid_device *hdev,
struct hid_input *hi,
* HID_DG_CONTACTCOUNT from the pen report as it is outside the physical
* collection, but within the report ID.
*/
if (field->physical == HID_DG_STYLUS)
return 0;
- else if ((field->physical == 0) &&
+
+ if ((field->physical == 0) &&
(field->report->id != td->mt_report_id) &&
(td->mt_report_id != -1))
return 0;
if (field->application == HID_DG_TOUCHSCREEN ||
--
2.18.0
Re: [PATCH 1/4] HID: multitouch: add MT_QUIRK_NOT_SEEN_MEANS_UP to MT_CLS_WIN_8 quirks
On Tue, Jul 03, 2018 at 09:53:19AM +0200, Benjamin Tissoires wrote:
> NACK on this.
> If the Surface has a buggy firmware, we should handle it as a special
> case, not as a common failure.
> Also, I am not sure this quirk is compatible with Win 8 specification.
> Last, we now have a timeout for unreleased touches, so I really don't
> think you need that in recent kernels.
>
> Cheers,
> Benjamin
>
> > { .name = MT_CLS_EXPORT_ALL_INPUTS,
> > .quirks = MT_QUIRK_ALWAYS_VALID |
> > MT_QUIRK_CONTACT_CNT_ACCURATE,
> > .export_all_inputs = true },
> > { .name = MT_CLS_WIN_8_DUAL,
> > --
> > 2.18.0
> >
Ah shoot, I had completely missed this reply to my earlier patchset, I
apologize. Now that you mention it, I think handling it as a special
case would be a better way to handle it, so I'll do a bit more research
on the quirk thing and then revise my patches (assuming they end up
as something still worth sending). Appreciate the comments, thanks.
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH 4/4] HID: multitouch: remove unneeded else conditional cases
On Tue, Jul 03, 2018 at 10:17:58AM +0200, Benjamin Tissoires wrote: > This one looks good. > Reviewed-by: Benjamin Tissoires > > Just FYI, I sent out a big refactor of the hid-multitouch code. Jiri > should be still reviewing it, so I am not so sure who will have to > rebase which series on top of the other, but someone between us will > have to do it :) Somehow also missed this reply... I guess I messed up my LKML filters somewhere. Noted, thanks for the review; I'll check your trees for the refactor when (if) I do my v2. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH 3/4] HID: multitouch: drop reports containing invalid values
On Tue, Jul 03, 2018 at 10:13:54AM +0200, Benjamin Tissoires wrote: > Hi Joey, > You can't really use plain values like that. There is a tiny chance > these values are valid on an other device. > IIRC, MS spec says that we should ignore out of band values if they > are tagged as such. Such input are tagged with NULL values > (http://www.usb.org/developers/hidpage/HID1_11.pdf page 31) and MS > spec mentioned this. > > All in all, if you have this bit set, you need to compare the value > with the logical_max/min for each field. > > I never encountered a device that required this, so you are probably > the lucky one :) Ah, you are completely right. After giving that pdf a read over I will definitely be dropping this patch from the v2. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH 2/4] HID: multitouch: don't check HID_GROUP_MULTITOUCH_WIN_8 for serial protocol
On Tue, Jul 03, 2018 at 10:16:01AM +0200, Benjamin Tissoires wrote: > There is a tiny difference between the HID group (this device looks > like it is used as a Win 8 device) and the device class (this is > effectively a Win8 device) > > It makes sense to remove this check for Win8 devices, but I don't > think it should be done for all devices. > > All in all, it won't change much. Hm, that sounds sane. I'll do a bit more research on this and see if restricting it to just Win8 devices is simple enough to be worthwhile. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH RESEND 1/4] HID: multitouch: add MT_QUIRK_NOT_SEEN_MEANS_UP to MT_CLS_WIN_8 quirks
On Thu, Aug 09, 2018 at 01:39:16PM -1000, Joey Pabalinas wrote: > The firmware found in the touch screen of the Surface Pro 3 is slightly > buggy and occasionally doesn't send lift off reports for contacts; add > MT_QUIRK_NOT_SEEN_MEANS_UP to .quirks to compensate for the missed > reports. > > Signed-off-by: Joey Pabalinas Sorry, my mail filters are in need of some adjusting; I completely missed the review of the first send of this patchset :( Please disregard this resend, as I am going to revise it and submit a v2, thanks. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [NOMERGE] [RFC PATCH 00/12] erofs: introduce erofs file system
On Fri, Jun 01, 2018 at 11:28:07AM +0200, Richard Weinberger wrote: > > We also think of other candidate full names, such as > > Enhanced / Extented Read-only File System, all the names short for "erofs" > > are okay. > > TBH, I read "erofs" as "error fs". ;-) I thought the exact same thing; opened the thread and was a bit confused at first. In my opinion the name feels slightly misleading. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [NOMERGE] [RFC PATCH 00/12] erofs: introduce erofs file system
On Thu, Jul 26, 2018 at 02:55:11PM -1000, Joey Pabalinas wrote: > I thought the exact same thing; opened the thread and was a bit confused > at first. In my opinion the name feels slightly misleading. Oooh, reading more of the thread I see there is some fun wordplay going on which I didn't catch at first. I rescind my comment :) -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH v2] crypto: testmgr: change `guard` to unsigned char
On Fri, Jan 12, 2018 at 11:23:28PM +1100, Herbert Xu wrote: > > Patch applied. Thanks. No problem, cheers. -- Joey Pabalinas signature.asc Description: PGP signature
[PATCH v2] scripts/kconfig: cleanup symbol handling code
Many of the variable names in scripts/kconfig/symbol.c are
far too terse to the point of not at all identifying _what_
they are actually used for (`p` and `l` as a couple examples),
and overall there is a large amount of code that could use
some cleaning up.
Give more explicit names to these variables, fix a couple cases
where different variables were sharing the same name and shadowing
each other, and overall cleanup a bit of the messiness in
sym_expand_string_value() and sym_escape_string_value()
while maintaining equivalent program behavior.
Suggested-by: Ulf Magnusson
Signed-off-by: Joey Pabalinas
1 file changed, 69 insertions(+), 61 deletions(-)
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 2220bc4b051bd914e3..9ee32ddb44e193719c 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -5,8 +5,8 @@
#include
#include
-#include
#include
+#include
#include
#include "lkc.h"
@@ -337,7 +337,7 @@ void sym_calc_value(struct symbol *sym)
{
struct symbol_value newval, oldval;
struct property *prop;
- struct expr *e;
+ struct expr *expr;
if (!sym)
return;
@@ -469,7 +469,7 @@ void sym_calc_value(struct symbol *sym)
struct symbol *choice_sym;
prop = sym_get_choice_prop(sym);
- expr_list_for_each_sym(prop->expr, e, choice_sym) {
+ expr_list_for_each_sym(prop->expr, expr, choice_sym) {
if ((sym->flags & SYMBOL_WRITE) &&
choice_sym->visible != no)
choice_sym->flags |= SYMBOL_WRITE;
@@ -899,94 +899,100 @@ struct symbol *sym_find(const char *name)
* name to be expanded shall be prefixed by a '$'. Unknown symbol expands to
* the empty string.
*/
-char *sym_expand_string_value(const char *in)
+char *sym_expand_string_value(const char *src)
{
- const char *src;
- char *res;
- size_t reslen;
+ const char *in;
+ char *res, *out;
+ size_t res_len, src_len;
/*
-* Note: 'in' might come from a token that's about to be
+* Note: 'src' might come from a token that'src about to be
* freed, so make sure to always allocate a new string
*/
- reslen = strlen(in) + 1;
- res = xmalloc(reslen);
- res[0] = '\0';
+ res_len = strlen(src) + 1;
+ res = xmalloc(res_len);
+ out = res;
- while ((src = strchr(in, '$'))) {
+ while ((in = strchr(src, '$'))) {
char *p, name[SYMBOL_MAXLENGTH];
- const char *symval = "";
+ const char *sym_val = "";
struct symbol *sym;
- size_t newlen;
+ size_t new_len, sym_len;
- strncat(res, in, src - in);
- src++;
+ strscpy(out, src, in - src);
+ out += in - src;
+ in++;
p = name;
- while (isalnum(*src) || *src == '_')
- *p++ = *src++;
+ while (isalnum(*in) || *in == '_')
+ *p++ = *in++;
*p = '\0';
sym = sym_find(name);
if (sym != NULL) {
sym_calc_value(sym);
- symval = sym_get_string_value(sym);
+ sym_val = sym_get_string_value(sym);
}
- newlen = strlen(res) + strlen(symval) + strlen(src) + 1;
- if (newlen > reslen) {
- reslen = newlen;
- res = xrealloc(res, reslen);
+ sym_len = strlen(sym_val);
+ new_len = sym_len + strlen(res) + strlen(in) + 1;
+ if (new_len > res_len) {
+ res_len = new_len;
+ res = xrealloc(res, res_len);
}
- strcat(res, symval);
- in = src;
+ strscpy(out, sym_val, sym_len);
+ out += sym_len;
+ src = in;
}
- strcat(res, in);
+ src_len = strlen(src);
+ strscpy(out, src, src_len);
+ out += src_len;
+ *out = '\0';
return res;
}
-const char *sym_escape_string_value(const char *in)
+const char *sym_escape_string_value(const char *src)
{
- const char *p;
- size_t reslen;
- char *res;
- size_t l;
+ const char *in;
+ size_t res_len, in_len;
+ char *res, *out;
- reslen = strlen(in) + strlen("\"\"") + 1;
+ res_len = strlen(src) + strlen("\"\"") + 1;
- p = in;
+ in = src;
for (;;) {
- l = strcspn(p, "\"\\");
- p
Re: [PATCH] tty/nozomi: refactor macros and functions
On Fri, Mar 23, 2018 at 04:27:37PM +0100, Greg Kroah-Hartman wrote: > That's a lot of different things to do all in a single patch. > > Please break this up into a patch series, doing only one logical "thing" > per patch. Sorry about that, I'll spin up a patchset now. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
[PATCH 4/4] tty/nozomi: refactor conditional statements
Reduce unnecessarily deep nesting of blocks and simplify
control flow (e.g. "if/else" constructs changed to "if/return"
and single case "switch" statements changed to "if" conditionals
where possible).
Signed-off-by: Joey Pabalinas
1 file changed, 52 insertions(+), 50 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index a5074a59d3e3d33e68..0ea3e1de23c093e808 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -694,12 +694,13 @@ static void enable_transmit_ul(enum port_type port,
struct nozomi *dc)
{
static const u16 mask[] = {MDM_UL, DIAG_UL, APP1_UL, APP2_UL, CTRL_UL};
- if (port < NOZOMI_MAX_PORTS) {
- dc->last_ier |= mask[port];
- writew(dc->last_ier, dc->reg_ier);
- } else {
+ if (port >= NOZOMI_MAX_PORTS) {
dev_err(&dc->pdev->dev, "Called with wrong port?\n");
+ return;
}
+
+ dc->last_ier |= mask[port];
+ writew(dc->last_ier, dc->reg_ier);
}
/* Disable uplink interrupts */
@@ -708,12 +709,13 @@ static void disable_transmit_ul(enum port_type port,
struct nozomi *dc)
static const u16 mask[] =
{~MDM_UL, ~DIAG_UL, ~APP1_UL, ~APP2_UL, ~CTRL_UL};
- if (port < NOZOMI_MAX_PORTS) {
- dc->last_ier &= mask[port];
- writew(dc->last_ier, dc->reg_ier);
- } else {
+ if (port >= NOZOMI_MAX_PORTS) {
dev_err(&dc->pdev->dev, "Called with wrong port?\n");
+ return;
}
+
+ dc->last_ier &= mask[port];
+ writew(dc->last_ier, dc->reg_ier);
}
/* Enable downlink interrupts */
@@ -721,12 +723,13 @@ static void enable_transmit_dl(enum port_type port,
struct nozomi *dc)
{
static const u16 mask[] = {MDM_DL, DIAG_DL, APP1_DL, APP2_DL, CTRL_DL};
- if (port < NOZOMI_MAX_PORTS) {
- dc->last_ier |= mask[port];
- writew(dc->last_ier, dc->reg_ier);
- } else {
+ if (port >= NOZOMI_MAX_PORTS) {
dev_err(&dc->pdev->dev, "Called with wrong port?\n");
+ return;
}
+
+ dc->last_ier |= mask[port];
+ writew(dc->last_ier, dc->reg_ier);
}
/* Disable downlink interrupts */
@@ -735,12 +738,13 @@ static void disable_transmit_dl(enum port_type port,
struct nozomi *dc)
static const u16 mask[] =
{~MDM_DL, ~DIAG_DL, ~APP1_DL, ~APP2_DL, ~CTRL_DL};
- if (port < NOZOMI_MAX_PORTS) {
- dc->last_ier &= mask[port];
- writew(dc->last_ier, dc->reg_ier);
- } else {
+ if (port >= NOZOMI_MAX_PORTS) {
dev_err(&dc->pdev->dev, "Called with wrong port?\n");
+ return;
}
+
+ dc->last_ier &= mask[port];
+ writew(dc->last_ier, dc->reg_ier);
}
/*
@@ -1028,33 +1032,31 @@ static int handle_data_dl(struct nozomi *dc, enum
port_type port, u8 *toggle,
if (*toggle == 0 && read_iir & mask1) {
if (receive_data(port, dc)) {
writew(mask1, dc->reg_fcr);
- *toggle = !(*toggle);
+ *toggle = !*toggle;
}
- if (read_iir & mask2) {
- if (receive_data(port, dc)) {
- writew(mask2, dc->reg_fcr);
- *toggle = !(*toggle);
- }
+ if (read_iir & mask2 && receive_data(port, dc)) {
+ writew(mask2, dc->reg_fcr);
+ *toggle = !*toggle;
}
+
+ return 1;
} else if (*toggle == 1 && read_iir & mask2) {
if (receive_data(port, dc)) {
writew(mask2, dc->reg_fcr);
- *toggle = !(*toggle);
+ *toggle = !*toggle;
}
- if (read_iir & mask1) {
- if (receive_data(port, dc)) {
- writew(mask1, dc->reg_fcr);
- *toggle = !(*toggle);
- }
+ if (read_iir & mask1 && receive_data(port, dc)) {
+ writew(mask1, dc->reg_fcr);
+ *toggle = !*toggle;
}
- } else {
- dev_err(&dc->pdev->dev, "port out of sync!, toggle:%d\n",
- *toggle);
- return 0;
+
+ return 1;
}
- return 1;
+
+ dev_err(&dc->pdev->dev, "port out of sync!, toggle:%d\n", *toggle);
+ return 0;
}
/*
@@ -1087,6 +1089,7 @@ stati
[PATCH 1/4] tty/nozomi: cleanup DUMP() macro
Replace snprint() with strscpy() and use max_t() instead of
the conditional operator.
Signed-off-by: Joey Pabalinas
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index b57b35066ebea94639..f26bf1d1e9ee0e74eb 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -72,19 +72,19 @@ do {
\
#define TMP_BUF_MAX 256
-#define DUMP(buf__,len__) \
- do { \
-char tbuf[TMP_BUF_MAX] = {0};\
-if (len__ > 1) {\
- snprintf(tbuf, len__ > TMP_BUF_MAX ? TMP_BUF_MAX : len__, "%s", buf__);\
- if (tbuf[len__-2] == '\r') {\
- tbuf[len__-2] = 'r';\
- } \
- DBG1("SENDING: '%s' (%d+n)", tbuf, len__);\
-} else {\
- DBG1("SENDING: '%s' (%d)", tbuf, len__);\
-} \
-} while (0)
+#define DUMP(buf__, len__) \
+ do {\
+ char tbuf[TMP_BUF_MAX] = {0}; \
+ if (len__ > 1) {\
+ u32 data_len = min_t(u32, len__, TMP_BUF_MAX); \
+ strscpy(tbuf, buf__, data_len); \
+ if (tbuf[data_len - 2] == '\r') \
+ tbuf[data_len - 2] = 'r'; \
+ DBG1("SENDING: '%s' (%d+n)", tbuf, len__); \
+ } else {\
+ DBG1("SENDING: '%s' (%d)", tbuf, len__);\
+ } \
+ } while (0)
/*Defines */
#define NOZOMI_NAME"nozomi"
--
2.16.3
[PATCH 3/4] tty/nozomi: improve code readability and style
Improve code clarity by renaming identifiers and reorganizing
function control flow.
Signed-off-by: Joey Pabalinas
1 file changed, 92 insertions(+), 91 deletions(-)
1 file changed, 76 insertions(+), 90 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index 0fcb4db721d2a42f08..a5074a59d3e3d33e68 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -401,7 +401,7 @@ static inline struct port *get_port_by_tty(const struct
tty_struct *tty)
static void read_mem32(u32 *buf, const void __iomem *mem_addr_start,
u32 size_bytes)
{
- u32 i = 0;
+ u32 nread = 0;
const u32 __iomem *ptr = mem_addr_start;
u16 *buf16;
@@ -411,30 +411,27 @@ static void read_mem32(u32 *buf, const void __iomem
*mem_addr_start,
/* shortcut for extremely often used cases */
switch (size_bytes) {
case 2: /* 2 bytes */
- buf16 = (u16 *) buf;
+ buf16 = (u16 *)buf;
*buf16 = __le16_to_cpu(readw(ptr));
goto out;
- break;
case 4: /* 4 bytes */
- *(buf) = __le32_to_cpu(readl(ptr));
+ *buf = __le32_to_cpu(readl(ptr));
goto out;
- break;
}
- while (i < size_bytes) {
- if (size_bytes - i == 2) {
+ for (; nread < size_bytes; buf++, ptr++) {
+ if (size_bytes - nread == 2) {
/* Handle 2 bytes in the end */
- buf16 = (u16 *) buf;
- *(buf16) = __le16_to_cpu(readw(ptr));
- i += 2;
+ buf16 = (u16 *)buf;
+ *buf16 = __le16_to_cpu(readw(ptr));
+ nread += 2;
} else {
/* Read 4 bytes */
- *(buf) = __le32_to_cpu(readl(ptr));
- i += 4;
+ *buf = __le32_to_cpu(readl(ptr));
+ nread += 4;
}
- buf++;
- ptr++;
}
+
out:
return;
}
@@ -447,7 +444,7 @@ static void read_mem32(u32 *buf, const void __iomem
*mem_addr_start,
static u32 write_mem32(void __iomem *mem_addr_start, const u32 *buf,
u32 size_bytes)
{
- u32 i = 0;
+ u32 nwritten = 0;
u32 __iomem *ptr = mem_addr_start;
const u16 *buf16;
@@ -460,7 +457,6 @@ static u32 write_mem32(void __iomem *mem_addr_start, const
u32 *buf,
buf16 = (const u16 *)buf;
writew(__cpu_to_le16(*buf16), ptr);
return 2;
- break;
case 1: /*
* also needs to write 4 bytes in this case
* so falling through..
@@ -468,24 +464,22 @@ static u32 write_mem32(void __iomem *mem_addr_start,
const u32 *buf,
case 4: /* 4 bytes */
writel(__cpu_to_le32(*buf), ptr);
return 4;
- break;
}
- while (i < size_bytes) {
- if (size_bytes - i == 2) {
+ for (; nwritten < size_bytes; buf++, ptr++) {
+ if (size_bytes - nwritten == 2) {
/* 2 bytes */
buf16 = (const u16 *)buf;
writew(__cpu_to_le16(*buf16), ptr);
- i += 2;
+ nwritten += 2;
} else {
/* 4 bytes */
writel(__cpu_to_le32(*buf), ptr);
- i += 4;
+ nwritten += 4;
}
- buf++;
- ptr++;
}
- return i;
+
+ return nwritten;
}
/* Setup pointers to different channels and also setup buffer sizes. */
@@ -632,9 +626,10 @@ static int nozomi_read_config_table(struct nozomi *dc)
return 0;
}
- if ((dc->config_table.version == 0)
- || (dc->config_table.toggle.enabled == TOGGLE_VALID)) {
+ if (!dc->config_table.version
+ || dc->config_table.toggle.enabled == TOGGLE_VALID) {
int i;
+
DBG1("Second phase, configuring card");
nozomi_setup_memory(dc);
@@ -659,12 +654,14 @@ static int nozomi_read_config_table(struct nozomi *dc)
dc->state = NOZOMI_STATE_ALLOCATED;
dev_info(&dc->pdev->dev, "Initialization OK!\n");
+
return 1;
}
- if ((dc->config_table.version > 0)
- && (dc->config_table.toggle.enabled != TOGGLE_VALID)) {
+ if (dc->config_table.version > 0
+ && dc->config_table.toggle.enabled != TOGGLE_VALID) {
u32 offset = 0;
+
DBG1("First phase: pushing upload buffers, clearing download")
[PATCH 2/4] tty/nozomi: fix inconsistent indentation
Correct misaligned indentation and remove extraneous spaces.
Signed-off-by: Joey Pabalinas
1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index f26bf1d1e9ee0e74eb..0fcb4db721d2a42f08 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -102,41 +102,41 @@ do {
\
#define RECEIVE_BUF_MAX4
-#define R_IIR 0x /* Interrupt Identity Register */
-#define R_FCR 0x /* Flow Control Register */
-#define R_IER 0x0004 /* Interrupt Enable Register */
+#define R_IIR 0x /* Interrupt Identity Register */
+#define R_FCR 0x /* Flow Control Register */
+#define R_IER 0x0004 /* Interrupt Enable Register */
#define NOZOMI_CONFIG_MAGIC0xEFEFFEFE
#define TOGGLE_VALID 0x
/* Definition of interrupt tokens */
-#define MDM_DL10x0001
-#define MDM_UL10x0002
-#define MDM_DL20x0004
-#define MDM_UL20x0008
-#define DIAG_DL1 0x0010
-#define DIAG_DL2 0x0020
-#define DIAG_UL0x0040
-#define APP1_DL0x0080
-#define APP1_UL0x0100
-#define APP2_DL0x0200
-#define APP2_UL0x0400
-#define CTRL_DL0x0800
-#define CTRL_UL0x1000
-#define RESET 0x8000
+#define MDM_DL10x0001
+#define MDM_UL10x0002
+#define MDM_DL20x0004
+#define MDM_UL20x0008
+#define DIAG_DL1 0x0010
+#define DIAG_DL2 0x0020
+#define DIAG_UL0x0040
+#define APP1_DL0x0080
+#define APP1_UL0x0100
+#define APP2_DL0x0200
+#define APP2_UL0x0400
+#define CTRL_DL0x0800
+#define CTRL_UL0x1000
+#define RESET 0x8000
-#define MDM_DL (MDM_DL1 | MDM_DL2)
-#define MDM_UL (MDM_UL1 | MDM_UL2)
-#define DIAG_DL(DIAG_DL1 | DIAG_DL2)
+#define MDM_DL (MDM_DL1 | MDM_DL2)
+#define MDM_UL (MDM_UL1 | MDM_UL2)
+#define DIAG_DL(DIAG_DL1 | DIAG_DL2)
/* modem signal definition */
-#define CTRL_DSR 0x0001
-#define CTRL_DCD 0x0002
-#define CTRL_RI0x0004
-#define CTRL_CTS 0x0008
+#define CTRL_DSR 0x0001
+#define CTRL_DCD 0x0002
+#define CTRL_RI0x0004
+#define CTRL_CTS 0x0008
-#define CTRL_DTR 0x0001
-#define CTRL_RTS 0x0002
+#define CTRL_DTR 0x0001
+#define CTRL_RTS 0x0002
#define MAX_PORT 4
#define NOZOMI_MAX_PORTS 5
@@ -365,7 +365,7 @@ struct buffer {
u8 *data;
} __attribute__ ((packed));
-/*Global variables */
+/* Global variables */
static const struct pci_device_id nozomi_pci_tbl[] = {
{PCI_DEVICE(0x1931, 0x000c)}, /* Nozomi HSDPA */
{},
@@ -1686,12 +1686,12 @@ static int ntty_tiocmget(struct tty_struct *tty)
/* Note: these could change under us but it is not clear this
matters if so */
- return (ctrl_ul->RTS ? TIOCM_RTS : 0) |
- (ctrl_ul->DTR ? TIOCM_DTR : 0) |
- (ctrl_dl->DCD ? TIOCM_CAR : 0) |
- (ctrl_dl->RI ? TIOCM_RNG : 0) |
- (ctrl_dl->DSR ? TIOCM_DSR : 0) |
- (ctrl_dl->CTS ? TIOCM_CTS : 0);
+ return (ctrl_ul->RTS ? TIOCM_RTS : 0)
+ | (ctrl_ul->DTR ? TIOCM_DTR : 0)
+ | (ctrl_dl->DCD ? TIOCM_CAR : 0)
+ | (ctrl_dl->RI ? TIOCM_RNG : 0)
+ | (ctrl_dl->DSR ? TIOCM_DSR : 0)
+ | (ctrl_dl->CTS ? TIOCM_CTS : 0);
}
/* Sets io controls parameters */
@@ -1722,10 +1722,10 @@ static int ntty_cflags_changed(struct port *port,
unsigned long flags,
const struct async_icount cnow = port->tty_icount;
int ret;
- ret = ((flags & TIOCM_RNG) && (cnow.rng != cprev->rng)) ||
- ((flags & TIOCM_DSR) && (cnow.dsr != cprev->dsr)) ||
- ((flags & TIOCM_CD) && (cnow.dcd != cprev->dcd)) ||
- ((flags & TIOCM_CTS) && (cnow.cts != cprev->cts));
+ ret = ((flags & TIOCM_RNG) && (cnow.rng != cprev->rng))
+ || ((flags & TIOCM_DSR) && (cnow.dsr != cprev->dsr))
+ || ((flags & TIOCM_CD) && (cnow.dcd != cprev->dcd))
+ || ((flags & TIOCM_CTS) && (cnow.cts != cprev->cts));
*cprev = cnow;
--
2.16.3
[PATCH 0/4] tty/nozomi: general module cleanup
The nozomi module code has a fair amount of sections which could use a bit of improvement; both style and clarity could be improved while maintaining equivalent semantics. Cleanup messy portions of the module code while preserving existing behavior by: - Replacing constructs like `len__ > TMP_BUF_MAX ? TMP_BUF_MAX : len__` with `min_t(u32, len__, TMP_BUF_MAX)` and function calls like snprintf(tbuf, ..., "%s", ...). with strscpy(tbuf, ..., ...). - Rename identifiers more descriptively (where appropriate). - Simplify deeply nested conditionals by replacing them with shallower (but semantically equivalent) logic. - Coalesce return paths / loop conditionals. - Remove pointless initializations and redundant parentheses/break statements. - Correct inconsistently indented lines and extraneous whitespace. CC: Greg Kroah-Hartman CC: Arnd Bergmann CC: Jiri Slaby CC: Tomasz Kramkowsk Joey Pabalinas (4): tty/nozomi: cleanup DUMP() macro tty/nozomi: fix inconsistent indentation tty/nozomi: improve code readability and style tty/nozomi: refactor conditional statements drivers/tty/nozomi.c | 362 +-- 1 file changed, 181 insertions(+), 181 deletions(-) -- 2.16.3
[PATCH v2 1/4] tty/nozomi: cleanup DUMP() macro
Replace snprint() with strscpy() and use max_t() instead of
the conditional operator.
Signed-off-by: Joey Pabalinas
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index b57b35066ebea94639..f26bf1d1e9ee0e74eb 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -72,19 +72,19 @@ do {
\
#define TMP_BUF_MAX 256
-#define DUMP(buf__,len__) \
- do { \
-char tbuf[TMP_BUF_MAX] = {0};\
-if (len__ > 1) {\
- snprintf(tbuf, len__ > TMP_BUF_MAX ? TMP_BUF_MAX : len__, "%s", buf__);\
- if (tbuf[len__-2] == '\r') {\
- tbuf[len__-2] = 'r';\
- } \
- DBG1("SENDING: '%s' (%d+n)", tbuf, len__);\
-} else {\
- DBG1("SENDING: '%s' (%d)", tbuf, len__);\
-} \
-} while (0)
+#define DUMP(buf__, len__) \
+ do {\
+ char tbuf[TMP_BUF_MAX] = {0}; \
+ if (len__ > 1) {\
+ u32 data_len = min_t(u32, len__, TMP_BUF_MAX); \
+ strscpy(tbuf, buf__, data_len); \
+ if (tbuf[data_len - 2] == '\r') \
+ tbuf[data_len - 2] = 'r'; \
+ DBG1("SENDING: '%s' (%d+n)", tbuf, len__); \
+ } else {\
+ DBG1("SENDING: '%s' (%d)", tbuf, len__);\
+ } \
+ } while (0)
/*Defines */
#define NOZOMI_NAME"nozomi"
--
2.16.3
[PATCH v2 4/4] tty/nozomi: refactor conditional statements
Reduce unnecessarily deep nesting of blocks and simplify
control flow (e.g. "if/else" constructs changed to "if/return"
and single case "switch" statements changed to "if" conditionals
where possible).
Signed-off-by: Joey Pabalinas
1 file changed, 52 insertions(+), 50 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index a5074a59d3e3d33e68..0ea3e1de23c093e808 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -694,12 +694,13 @@ static void enable_transmit_ul(enum port_type port,
struct nozomi *dc)
{
static const u16 mask[] = {MDM_UL, DIAG_UL, APP1_UL, APP2_UL, CTRL_UL};
- if (port < NOZOMI_MAX_PORTS) {
- dc->last_ier |= mask[port];
- writew(dc->last_ier, dc->reg_ier);
- } else {
+ if (port >= NOZOMI_MAX_PORTS) {
dev_err(&dc->pdev->dev, "Called with wrong port?\n");
+ return;
}
+
+ dc->last_ier |= mask[port];
+ writew(dc->last_ier, dc->reg_ier);
}
/* Disable uplink interrupts */
@@ -708,12 +709,13 @@ static void disable_transmit_ul(enum port_type port,
struct nozomi *dc)
static const u16 mask[] =
{~MDM_UL, ~DIAG_UL, ~APP1_UL, ~APP2_UL, ~CTRL_UL};
- if (port < NOZOMI_MAX_PORTS) {
- dc->last_ier &= mask[port];
- writew(dc->last_ier, dc->reg_ier);
- } else {
+ if (port >= NOZOMI_MAX_PORTS) {
dev_err(&dc->pdev->dev, "Called with wrong port?\n");
+ return;
}
+
+ dc->last_ier &= mask[port];
+ writew(dc->last_ier, dc->reg_ier);
}
/* Enable downlink interrupts */
@@ -721,12 +723,13 @@ static void enable_transmit_dl(enum port_type port,
struct nozomi *dc)
{
static const u16 mask[] = {MDM_DL, DIAG_DL, APP1_DL, APP2_DL, CTRL_DL};
- if (port < NOZOMI_MAX_PORTS) {
- dc->last_ier |= mask[port];
- writew(dc->last_ier, dc->reg_ier);
- } else {
+ if (port >= NOZOMI_MAX_PORTS) {
dev_err(&dc->pdev->dev, "Called with wrong port?\n");
+ return;
}
+
+ dc->last_ier |= mask[port];
+ writew(dc->last_ier, dc->reg_ier);
}
/* Disable downlink interrupts */
@@ -735,12 +738,13 @@ static void disable_transmit_dl(enum port_type port,
struct nozomi *dc)
static const u16 mask[] =
{~MDM_DL, ~DIAG_DL, ~APP1_DL, ~APP2_DL, ~CTRL_DL};
- if (port < NOZOMI_MAX_PORTS) {
- dc->last_ier &= mask[port];
- writew(dc->last_ier, dc->reg_ier);
- } else {
+ if (port >= NOZOMI_MAX_PORTS) {
dev_err(&dc->pdev->dev, "Called with wrong port?\n");
+ return;
}
+
+ dc->last_ier &= mask[port];
+ writew(dc->last_ier, dc->reg_ier);
}
/*
@@ -1028,33 +1032,31 @@ static int handle_data_dl(struct nozomi *dc, enum
port_type port, u8 *toggle,
if (*toggle == 0 && read_iir & mask1) {
if (receive_data(port, dc)) {
writew(mask1, dc->reg_fcr);
- *toggle = !(*toggle);
+ *toggle = !*toggle;
}
- if (read_iir & mask2) {
- if (receive_data(port, dc)) {
- writew(mask2, dc->reg_fcr);
- *toggle = !(*toggle);
- }
+ if (read_iir & mask2 && receive_data(port, dc)) {
+ writew(mask2, dc->reg_fcr);
+ *toggle = !*toggle;
}
+
+ return 1;
} else if (*toggle == 1 && read_iir & mask2) {
if (receive_data(port, dc)) {
writew(mask2, dc->reg_fcr);
- *toggle = !(*toggle);
+ *toggle = !*toggle;
}
- if (read_iir & mask1) {
- if (receive_data(port, dc)) {
- writew(mask1, dc->reg_fcr);
- *toggle = !(*toggle);
- }
+ if (read_iir & mask1 && receive_data(port, dc)) {
+ writew(mask1, dc->reg_fcr);
+ *toggle = !*toggle;
}
- } else {
- dev_err(&dc->pdev->dev, "port out of sync!, toggle:%d\n",
- *toggle);
- return 0;
+
+ return 1;
}
- return 1;
+
+ dev_err(&dc->pdev->dev, "port out of sync!, toggle:%d\n", *toggle);
+ return 0;
}
/*
@@ -1087,6 +1089,7 @@ stati
[PATCH v2 2/4] tty/nozomi: fix inconsistent indentation
Correct misaligned indentation and remove extraneous spaces.
Signed-off-by: Joey Pabalinas
1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index f26bf1d1e9ee0e74eb..0fcb4db721d2a42f08 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -102,41 +102,41 @@ do {
\
#define RECEIVE_BUF_MAX4
-#define R_IIR 0x /* Interrupt Identity Register */
-#define R_FCR 0x /* Flow Control Register */
-#define R_IER 0x0004 /* Interrupt Enable Register */
+#define R_IIR 0x /* Interrupt Identity Register */
+#define R_FCR 0x /* Flow Control Register */
+#define R_IER 0x0004 /* Interrupt Enable Register */
#define NOZOMI_CONFIG_MAGIC0xEFEFFEFE
#define TOGGLE_VALID 0x
/* Definition of interrupt tokens */
-#define MDM_DL10x0001
-#define MDM_UL10x0002
-#define MDM_DL20x0004
-#define MDM_UL20x0008
-#define DIAG_DL1 0x0010
-#define DIAG_DL2 0x0020
-#define DIAG_UL0x0040
-#define APP1_DL0x0080
-#define APP1_UL0x0100
-#define APP2_DL0x0200
-#define APP2_UL0x0400
-#define CTRL_DL0x0800
-#define CTRL_UL0x1000
-#define RESET 0x8000
+#define MDM_DL10x0001
+#define MDM_UL10x0002
+#define MDM_DL20x0004
+#define MDM_UL20x0008
+#define DIAG_DL1 0x0010
+#define DIAG_DL2 0x0020
+#define DIAG_UL0x0040
+#define APP1_DL0x0080
+#define APP1_UL0x0100
+#define APP2_DL0x0200
+#define APP2_UL0x0400
+#define CTRL_DL0x0800
+#define CTRL_UL0x1000
+#define RESET 0x8000
-#define MDM_DL (MDM_DL1 | MDM_DL2)
-#define MDM_UL (MDM_UL1 | MDM_UL2)
-#define DIAG_DL(DIAG_DL1 | DIAG_DL2)
+#define MDM_DL (MDM_DL1 | MDM_DL2)
+#define MDM_UL (MDM_UL1 | MDM_UL2)
+#define DIAG_DL(DIAG_DL1 | DIAG_DL2)
/* modem signal definition */
-#define CTRL_DSR 0x0001
-#define CTRL_DCD 0x0002
-#define CTRL_RI0x0004
-#define CTRL_CTS 0x0008
+#define CTRL_DSR 0x0001
+#define CTRL_DCD 0x0002
+#define CTRL_RI0x0004
+#define CTRL_CTS 0x0008
-#define CTRL_DTR 0x0001
-#define CTRL_RTS 0x0002
+#define CTRL_DTR 0x0001
+#define CTRL_RTS 0x0002
#define MAX_PORT 4
#define NOZOMI_MAX_PORTS 5
@@ -365,7 +365,7 @@ struct buffer {
u8 *data;
} __attribute__ ((packed));
-/*Global variables */
+/* Global variables */
static const struct pci_device_id nozomi_pci_tbl[] = {
{PCI_DEVICE(0x1931, 0x000c)}, /* Nozomi HSDPA */
{},
@@ -1686,12 +1686,12 @@ static int ntty_tiocmget(struct tty_struct *tty)
/* Note: these could change under us but it is not clear this
matters if so */
- return (ctrl_ul->RTS ? TIOCM_RTS : 0) |
- (ctrl_ul->DTR ? TIOCM_DTR : 0) |
- (ctrl_dl->DCD ? TIOCM_CAR : 0) |
- (ctrl_dl->RI ? TIOCM_RNG : 0) |
- (ctrl_dl->DSR ? TIOCM_DSR : 0) |
- (ctrl_dl->CTS ? TIOCM_CTS : 0);
+ return (ctrl_ul->RTS ? TIOCM_RTS : 0)
+ | (ctrl_ul->DTR ? TIOCM_DTR : 0)
+ | (ctrl_dl->DCD ? TIOCM_CAR : 0)
+ | (ctrl_dl->RI ? TIOCM_RNG : 0)
+ | (ctrl_dl->DSR ? TIOCM_DSR : 0)
+ | (ctrl_dl->CTS ? TIOCM_CTS : 0);
}
/* Sets io controls parameters */
@@ -1722,10 +1722,10 @@ static int ntty_cflags_changed(struct port *port,
unsigned long flags,
const struct async_icount cnow = port->tty_icount;
int ret;
- ret = ((flags & TIOCM_RNG) && (cnow.rng != cprev->rng)) ||
- ((flags & TIOCM_DSR) && (cnow.dsr != cprev->dsr)) ||
- ((flags & TIOCM_CD) && (cnow.dcd != cprev->dcd)) ||
- ((flags & TIOCM_CTS) && (cnow.cts != cprev->cts));
+ ret = ((flags & TIOCM_RNG) && (cnow.rng != cprev->rng))
+ || ((flags & TIOCM_DSR) && (cnow.dsr != cprev->dsr))
+ || ((flags & TIOCM_CD) && (cnow.dcd != cprev->dcd))
+ || ((flags & TIOCM_CTS) && (cnow.cts != cprev->cts));
*cprev = cnow;
--
2.16.3
[PATCH v2 0/4] tty/nozomi: general module cleanup
Sorry, totally screwed up git send-email CC's there; resending the patchset. The nozomi module code has a fair amount of sections which could use a bit of improvement; both style and clarity could be improved while maintaining equivalent semantics. Cleanup messy portions of the module code while preserving existing behavior by: - Replacing constructs like `len__ > TMP_BUF_MAX ? TMP_BUF_MAX : len__` with `min_t(u32, len__, TMP_BUF_MAX)` and function calls like snprintf(tbuf, ..., "%s", ...). with strscpy(tbuf, ..., ...). - Rename identifiers more descriptively (where appropriate). - Simplify deeply nested conditionals by replacing them with shallower (but semantically equivalent) logic. - Coalesce return paths / loop conditionals. - Remove pointless initializations and redundant parentheses/break statements. - Correct inconsistently indented lines and extraneous whitespace. CC: Greg Kroah-Hartman CC: Arnd Bergmann CC: Jiri Slaby CC: Tomasz Kramkowsk Joey Pabalinas (4): tty/nozomi: cleanup DUMP() macro tty/nozomi: fix inconsistent indentation tty/nozomi: improve code readability and style tty/nozomi: refactor conditional statements drivers/tty/nozomi.c | 362 +-- 1 file changed, 181 insertions(+), 181 deletions(-) -- 2.16.3
[PATCH v2 3/4] tty/nozomi: improve code readability and style
Improve code clarity by renaming identifiers and reorganizing
function control flow.
Signed-off-by: Joey Pabalinas
1 file changed, 92 insertions(+), 91 deletions(-)
1 file changed, 76 insertions(+), 90 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index 0fcb4db721d2a42f08..a5074a59d3e3d33e68 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -401,7 +401,7 @@ static inline struct port *get_port_by_tty(const struct
tty_struct *tty)
static void read_mem32(u32 *buf, const void __iomem *mem_addr_start,
u32 size_bytes)
{
- u32 i = 0;
+ u32 nread = 0;
const u32 __iomem *ptr = mem_addr_start;
u16 *buf16;
@@ -411,30 +411,27 @@ static void read_mem32(u32 *buf, const void __iomem
*mem_addr_start,
/* shortcut for extremely often used cases */
switch (size_bytes) {
case 2: /* 2 bytes */
- buf16 = (u16 *) buf;
+ buf16 = (u16 *)buf;
*buf16 = __le16_to_cpu(readw(ptr));
goto out;
- break;
case 4: /* 4 bytes */
- *(buf) = __le32_to_cpu(readl(ptr));
+ *buf = __le32_to_cpu(readl(ptr));
goto out;
- break;
}
- while (i < size_bytes) {
- if (size_bytes - i == 2) {
+ for (; nread < size_bytes; buf++, ptr++) {
+ if (size_bytes - nread == 2) {
/* Handle 2 bytes in the end */
- buf16 = (u16 *) buf;
- *(buf16) = __le16_to_cpu(readw(ptr));
- i += 2;
+ buf16 = (u16 *)buf;
+ *buf16 = __le16_to_cpu(readw(ptr));
+ nread += 2;
} else {
/* Read 4 bytes */
- *(buf) = __le32_to_cpu(readl(ptr));
- i += 4;
+ *buf = __le32_to_cpu(readl(ptr));
+ nread += 4;
}
- buf++;
- ptr++;
}
+
out:
return;
}
@@ -447,7 +444,7 @@ static void read_mem32(u32 *buf, const void __iomem
*mem_addr_start,
static u32 write_mem32(void __iomem *mem_addr_start, const u32 *buf,
u32 size_bytes)
{
- u32 i = 0;
+ u32 nwritten = 0;
u32 __iomem *ptr = mem_addr_start;
const u16 *buf16;
@@ -460,7 +457,6 @@ static u32 write_mem32(void __iomem *mem_addr_start, const
u32 *buf,
buf16 = (const u16 *)buf;
writew(__cpu_to_le16(*buf16), ptr);
return 2;
- break;
case 1: /*
* also needs to write 4 bytes in this case
* so falling through..
@@ -468,24 +464,22 @@ static u32 write_mem32(void __iomem *mem_addr_start,
const u32 *buf,
case 4: /* 4 bytes */
writel(__cpu_to_le32(*buf), ptr);
return 4;
- break;
}
- while (i < size_bytes) {
- if (size_bytes - i == 2) {
+ for (; nwritten < size_bytes; buf++, ptr++) {
+ if (size_bytes - nwritten == 2) {
/* 2 bytes */
buf16 = (const u16 *)buf;
writew(__cpu_to_le16(*buf16), ptr);
- i += 2;
+ nwritten += 2;
} else {
/* 4 bytes */
writel(__cpu_to_le32(*buf), ptr);
- i += 4;
+ nwritten += 4;
}
- buf++;
- ptr++;
}
- return i;
+
+ return nwritten;
}
/* Setup pointers to different channels and also setup buffer sizes. */
@@ -632,9 +626,10 @@ static int nozomi_read_config_table(struct nozomi *dc)
return 0;
}
- if ((dc->config_table.version == 0)
- || (dc->config_table.toggle.enabled == TOGGLE_VALID)) {
+ if (!dc->config_table.version
+ || dc->config_table.toggle.enabled == TOGGLE_VALID) {
int i;
+
DBG1("Second phase, configuring card");
nozomi_setup_memory(dc);
@@ -659,12 +654,14 @@ static int nozomi_read_config_table(struct nozomi *dc)
dc->state = NOZOMI_STATE_ALLOCATED;
dev_info(&dc->pdev->dev, "Initialization OK!\n");
+
return 1;
}
- if ((dc->config_table.version > 0)
- && (dc->config_table.toggle.enabled != TOGGLE_VALID)) {
+ if (dc->config_table.version > 0
+ && dc->config_table.toggle.enabled != TOGGLE_VALID) {
u32 offset = 0;
+
DBG1("First phase: pushing upload buffers, clearing download")
Re: [PATCH v2] scripts/kconfig: cleanup symbol handling code
On Mon, Mar 26, 2018 at 01:52:26AM +0900, Masahiro Yamada wrote: > I want to see Kconfig improvements in a bigger picture. > > The changes below are noise. That's understandable; I do agree that nothing here is _fundamentally_ broken at all, so no worries. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
[PATCH v2] add -Wpointer-arith sparse flag to toggle sizeof(void) warnings
Recent changes to the min()/max() macros in include/linux/kernel.h
have added a lot of noise when compiling the kernel with Sparse checking
enabled. This mostly is due to the *huge* increase in the number of
sizeof(void) warnings, a larger number of which can safely be ignored.
Add the -Wpointer-arith flag to enable/disable these warnings (along
with the warning when applying sizeof to function types exactly like the
GCC -Wpointer-arith flag) on demand; the warning itself has been disabled
by default to reduce the large influx of noise which was inadvertently
added by commit 3c8ba0d61d04ced9f8 (kernel.h: Retain constant expression
output for max()/min()).
Update the manpage to document the new flag.
CC: Kees Cook
CC: Linus Torvalds
CC: Martin Uecker
CC: Al Viro
CC: Christopher Li
CC: Joey Pabalinas
CC: Luc Van Oostenryck
Signed-off-by: Joey Pabalinas
Signed-off-by: Luc Van Oostenryck
4 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/evaluate.c b/evaluate.c
index 4e1dffe9c5416380df..f828da37df8e686623 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2193,7 +2193,8 @@ static struct symbol *evaluate_sizeof(struct expression
*expr)
size = type->bit_size;
if (size < 0 && is_void_type(type)) {
- warning(expr->pos, "expression using sizeof(void)");
+ if (Wpointer_arith)
+ warning(expr->pos, "expression using sizeof(void)");
size = bits_in_char;
}
@@ -2204,7 +2205,8 @@ static struct symbol *evaluate_sizeof(struct expression
*expr)
}
if (is_function(type->ctype.base_type)) {
- warning(expr->pos, "expression using sizeof on a function");
+ if (Wpointer_arith)
+ warning(expr->pos, "expression using sizeof on a
function");
size = bits_in_char;
}
diff --git a/lib.c b/lib.c
index 73d372c36626538bab..f7fdac96674aec4c24 100644
--- a/lib.c
+++ b/lib.c
@@ -242,6 +242,7 @@ int Woverride_init = 1;
int Woverride_init_all = 0;
int Woverride_init_whole_range = 0;
int Wparen_string = 0;
+int Wpointer_arith = 0;
int Wptr_subtraction_blows = 0;
int Wreturn_void = 0;
int Wshadow = 0;
@@ -654,6 +655,7 @@ static const struct flag warnings[] = {
{ "return-void", &Wreturn_void },
{ "shadow", &Wshadow },
{ "sizeof-bool", &Wsizeof_bool },
+ { "pointer-arith", &Wpointer_arith },
{ "sparse-error", &Wsparse_error },
{ "tautological-compare", &Wtautological_compare },
{ "transparent-union", &Wtransparent_union },
diff --git a/lib.h b/lib.h
index 3050b5577ba4d42e97..e34bb9a02ebac03f52 100644
--- a/lib.h
+++ b/lib.h
@@ -151,6 +151,7 @@ extern int Woverride_init;
extern int Woverride_init_all;
extern int Woverride_init_whole_range;
extern int Wparen_string;
+extern int Wpointer_arith;
extern int Wptr_subtraction_blows;
extern int Wreturn_void;
extern int Wshadow;
diff --git a/sparse.1 b/sparse.1
index 88343f3170f195297a..4379406999c94b806e 100644
--- a/sparse.1
+++ b/sparse.1
@@ -288,6 +288,25 @@ Standard C syntax does not permit a parenthesized string
as an array
initializer. GCC allows this syntax as an extension. With
\fB\-Wparen\-string\fR, Sparse will warn about this syntax.
+Sparse does not issue these warnings by default.
+.
+.TP
+.B \-Wpointer-arith
+Warn about anything that depends on the \fBsizeof\fR a function type or of
void.
+
+C99 does not allow the \fBsizeof\fR operator to be applied to function types
or to
+incomplete types such as void. GCC allows \fBsizeof\fR to be applied to these
+types as an extension and assigns these types a size of \fI1\fR.
+
+Although non-standard (and somewhat illogical), constructs such as
\fBsizeof(void)\fR
+are often useful when the intent is to operate on an expression without
evaluating
+it, e.g. in the following integer constant expression predicate:
+
+.nf
+#define __is_constexpr(x) \\
+ (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
+.fi
+
Sparse does not issue these warnings by default.
.
.TP
--
2.17.0.rc1.35.g90bbd502d54fe92035.dirty
signature.asc
Description: PGP signature
[PATCH] ACPI: prefer bool over int for predicates
Prefer bool over int for variables / returns which are
predicate expressions to make it explicit that these
expressions are evaluating simple "yes or no?" queries.
This makes it more obvious which expressions are _not_
that simple and require more attention, e.g. an `int ret`
meant to hold 0 or -ENOENT as a return value or an
`unsigned nmemb` meant to refer to the number of valid
members in some arbitrary array.
Change relevant variable / return types from int to bool and
prefer a true / false value for predicate expressions versus
a plain 1 / 0 value.
Signed-off-by: Joey Pabalinas
drivers/acpi/battery.c | 4 ++--
drivers/acpi/ec.c | 20 +---
drivers/acpi/pci_root.c | 17 ++---
drivers/acpi/scan.c | 6 +++---
include/acpi/acpi_bus.h | 2 +-
5 files changed, 21 insertions(+), 28 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index bdb24d636d9acc9c1a..f1a5fb5252969f0478 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -1416,7 +1416,7 @@ static int acpi_battery_add(struct acpi_device *device)
battery->pm_nb.notifier_call = battery_notify;
register_pm_notifier(&battery->pm_nb);
- device_init_wakeup(&device->dev, 1);
+ device_init_wakeup(&device->dev, true);
return result;
@@ -1434,7 +1434,7 @@ static int acpi_battery_remove(struct acpi_device *device)
if (!device || !acpi_driver_data(device))
return -EINVAL;
- device_init_wakeup(&device->dev, 0);
+ device_init_wakeup(&device->dev, false);
battery = acpi_driver_data(device);
unregister_pm_notifier(&battery->pm_nb);
#ifdef CONFIG_ACPI_PROCFS_POWER
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 30a5729565575f83cb..d4a564ab9cdd53046c 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -350,7 +350,7 @@ static inline bool acpi_ec_is_gpe_raised(struct acpi_ec *ec)
acpi_event_status gpe_status = 0;
(void)acpi_get_gpe_status(NULL, ec->gpe, &gpe_status);
- return (gpe_status & ACPI_EVENT_FLAG_STATUS_SET) ? true : false;
+ return gpe_status & ACPI_EVENT_FLAG_STATUS_SET;
}
static inline void acpi_ec_enable_gpe(struct acpi_ec *ec, bool open)
@@ -580,28 +580,26 @@ static bool acpi_ec_guard_event(struct acpi_ec *ec)
return guarded;
}
-static int ec_transaction_polled(struct acpi_ec *ec)
+static bool ec_transaction_polled(struct acpi_ec *ec)
{
unsigned long flags;
- int ret = 0;
+ bool polled;
spin_lock_irqsave(&ec->lock, flags);
- if (ec->curr && (ec->curr->flags & ACPI_EC_COMMAND_POLL))
- ret = 1;
+ polled = ec->curr && (ec->curr->flags & ACPI_EC_COMMAND_POLL);
spin_unlock_irqrestore(&ec->lock, flags);
- return ret;
+ return polled;
}
-static int ec_transaction_completed(struct acpi_ec *ec)
+static bool ec_transaction_completed(struct acpi_ec *ec)
{
unsigned long flags;
- int ret = 0;
+ bool completed;
spin_lock_irqsave(&ec->lock, flags);
- if (ec->curr && (ec->curr->flags & ACPI_EC_COMMAND_COMPLETE))
- ret = 1;
+ completed = ec->curr && (ec->curr->flags & ACPI_EC_COMMAND_COMPLETE);
spin_unlock_irqrestore(&ec->lock, flags);
- return ret;
+ return completed;
}
static inline void ec_transaction_transition(struct acpi_ec *ec, unsigned long
flag)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 6fc204a524932e97f4..61c0c079cff346e492 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -81,20 +81,15 @@ static DEFINE_MUTEX(osc_lock);
* Note: we could make this API take a struct acpi_device * instead, but
* for now, it's more convenient to operate on an acpi_handle.
*/
-int acpi_is_root_bridge(acpi_handle handle)
+bool acpi_is_root_bridge(acpi_handle handle)
{
- int ret;
struct acpi_device *device;
- ret = acpi_bus_get_device(handle, &device);
- if (ret)
- return 0;
-
- ret = acpi_match_device_ids(device, root_device_ids);
- if (ret)
- return 0;
- else
- return 1;
+ if (acpi_bus_get_device(handle, &device))
+ return false;
+ if (acpi_match_device_ids(device, root_device_ids))
+ return false;
+ return true;
}
EXPORT_SYMBOL_GPL(acpi_is_root_bridge);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 490498eca0d3db7d6a..8e3d436184104b5799 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -625,7 +625,7 @@ int acpi_device_add(struct acpi_device *device,
{
int result;
struct acpi_device_bus_id *acpi_device_bus_id, *new_bus_id;
- int found = 0;
+ bool fo
[PATCH] tty/nozomi: refactor macros and functions
Cleanup a few messy sections of code by replacing constructs
like `len__ > TMP_BUF_MAX ? TMP_BUF_MAX : len__` with
`min_t(u32, len__, TMP_BUF_MAX)` and naming identifiers
more descriptively (where appropriate).
A few sections were nested pretty deeply and have been
replaced with shallower (but semantically equivalent) logic.
In addition, simplify and coalesce a few of the
return paths / loop conditionals and correct a few
pointless Initializations, redundant parentheses/break
statements, and inconsistently indented line.
Signed-off-by: Joey Pabalinas
1 file changed, 181 insertions(+), 181 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index b57b35066ebea94639..7b7474b8530d85e5d9 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -72,19 +72,19 @@ do {
\
#define TMP_BUF_MAX 256
-#define DUMP(buf__,len__) \
- do { \
-char tbuf[TMP_BUF_MAX] = {0};\
-if (len__ > 1) {\
- snprintf(tbuf, len__ > TMP_BUF_MAX ? TMP_BUF_MAX : len__, "%s", buf__);\
- if (tbuf[len__-2] == '\r') {\
- tbuf[len__-2] = 'r';\
- } \
- DBG1("SENDING: '%s' (%d+n)", tbuf, len__);\
-} else {\
- DBG1("SENDING: '%s' (%d)", tbuf, len__);\
-} \
-} while (0)
+#define DUMP(buf__, len__) \
+ do {\
+ char tbuf[TMP_BUF_MAX] = {0}; \
+ if (len__ > 1) {\
+ u32 data_len = min_t(u32, len__, TMP_BUF_MAX); \
+ strscpy(tbuf, buf__, data_len); \
+ if (tbuf[data_len - 2] == '\r') \
+ tbuf[data_len - 2] = 'r'; \
+ DBG1("SENDING: '%s' (%d+n)", tbuf, len__); \
+ } else {\
+ DBG1("SENDING: '%s' (%d)", tbuf, len__);\
+ } \
+ } while (0)
/*Defines */
#define NOZOMI_NAME"nozomi"
@@ -102,41 +102,41 @@ do {
\
#define RECEIVE_BUF_MAX4
-#define R_IIR 0x /* Interrupt Identity Register */
-#define R_FCR 0x /* Flow Control Register */
-#define R_IER 0x0004 /* Interrupt Enable Register */
+#define R_IIR 0x /* Interrupt Identity Register */
+#define R_FCR 0x /* Flow Control Register */
+#define R_IER 0x0004 /* Interrupt Enable Register */
#define NOZOMI_CONFIG_MAGIC0xEFEFFEFE
#define TOGGLE_VALID 0x
/* Definition of interrupt tokens */
-#define MDM_DL10x0001
-#define MDM_UL10x0002
-#define MDM_DL20x0004
-#define MDM_UL20x0008
-#define DIAG_DL1 0x0010
-#define DIAG_DL2 0x0020
-#define DIAG_UL0x0040
-#define APP1_DL0x0080
-#define APP1_UL0x0100
-#define APP2_DL0x0200
-#define APP2_UL0x0400
-#define CTRL_DL0x0800
-#define CTRL_UL0x1000
-#define RESET 0x8000
+#define MDM_DL10x0001
+#define MDM_UL10x0002
+#define MDM_DL20x0004
+#define MDM_UL20x0008
+#define DIAG_DL1 0x0010
+#define DIAG_DL2 0x0020
+#define DIAG_UL0x0040
+#define APP1_DL0x0080
+#define APP1_UL0x0100
+#define APP2_DL0x0200
+#define APP2_UL0x0400
+#define CTRL_DL0x0800
+#define CTRL_UL0x1000
+#define RESET 0x8000
-#define MDM_DL (MDM_DL1 | MDM_DL2)
-#define MDM_UL (MDM_UL1 | MDM_UL2)
-#define DIAG_DL(DIAG_DL1 | DIAG_DL2)
+#define MDM_DL (MDM_DL1 | MDM_DL2)
+#define MDM_UL (MDM_UL1 | MDM_UL2)
+#define DIAG_DL(DIAG_DL1 | DIAG_DL2)
/* modem signal definition */
-#define CTRL_DSR 0x0001
-#define CTRL_DCD 0x0002
-#define CTRL_RI0x0004
-#define CTRL_CTS 0x0008
+#define CTRL_DSR 0x0001
+#define CTRL_DCD 0x0002
+#define CTRL_RI0x0004
+#define CTRL_CTS 0x0008
-#define CTRL_DTR 0x0001
-#define CTRL_RTS 0x0002
+#define CTRL_DTR 0x0001
+#define
Re: [PATCH v2] add -Wpointer-arith sparse flag to toggle sizeof(void) warnings
On Sun, Apr 08, 2018 at 09:48:24AM +0200, Luc Van Oostenryck wrote: > With the warning disabled by default (for the moment), I had to adapt > the testsuite with: Ah, so should I include that change in the patch itself when I make a V3? > > +Warn about anything that depends on the \fBsizeof\fR a function type or of > > void. > > Maybe it would be useful to add something along the line of "like directly > using > the sizeof operator on void or doing pointer arithmetic on a void pointer" ? I actually just took the explanation straight from the GCC man page since I figured the explanation should match (as the flag itself is basicallt copied). But I do sort of like your wording of it more, so if no one else sees any reasons to _not_ to diverge from GCC's wording here I have no problem changing that. > > +Although non-standard (and somewhat illogical), constructs such as > > \fBsizeof(void)\fR > > +are often useful when the intent is to operate on an expression without > > evaluating > > +it, e.g. in the following integer constant expression predicate: > > +.nf > > +#define __is_constexpr(x) \\ > > + (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8))) > > +.fi > > I think that pointer arithmetic is much more useful than taking the size of > void > (being able to take the size of *any* thing is somewhere in the middle, IMO). > But in all case, I don't think this part should belong to the man page. Also have no problem eliding this section if no one else has any good arguments for keeping it. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH v2] add -Wpointer-arith sparse flag to toggle sizeof(void) warnings
On Mon, Apr 09, 2018 at 12:51:01PM -1000, Joey Pabalinas wrote: > Ah, so should I include that change in the patch itself when I make a V3? Sending a v3 now incorporating the suggestions. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
[PATCH v3] sparse: add -Wpointer-arith flag to toggle sizeof(void) warnings
Recent changes to the min()/max() macros in include/linux/kernel.h
have added a lot of noise when compiling the kernel with Sparse checking
enabled. This mostly is due to the *huge* increase in the number of
sizeof(void) warnings, a larger number of which can safely be ignored.
Add the -Wpointer-arith flag to enable/disable these warnings (along
with the warning when applying sizeof to function types as well as
warning about pointer arithmetic on these types exactly like the
GCC -Wpointer-arith flag) on demand; the warning itself has been disabled
by default to reduce the large influx of noise which was inadvertently
added by commit 3c8ba0d61d04ced9f8 (kernel.h: Retain constant expression
output for max()/min()).
Update the manpage to document the new flag and add/update validation
cases regarding the application of sizeof to void and function types.
CC: Kees Cook
CC: Linus Torvalds
CC: Martin Uecker
CC: Al Viro
CC: Christopher Li
CC: Joey Pabalinas
CC: Luc Van Oostenryck
Signed-off-by: Joey Pabalinas
Signed-off-by: Luc Van Oostenryck
evaluate.c | 6 --
lib.c| 2 ++
lib.h| 1 +
sparse.1 | 13
validation/sizeof-function.c | 2 +-
validation/sizeof-void.c | 39
6 files changed, 60 insertions(+), 3 deletions(-)
create mode 100644 validation/sizeof-void.c
6 files changed, 60 insertions(+), 3 deletions(-)
diff --git a/evaluate.c b/evaluate.c
index 4e1dffe9c5416380df..f828da37df8e686623 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2193,7 +2193,8 @@ static struct symbol *evaluate_sizeof(struct expression
*expr)
size = type->bit_size;
if (size < 0 && is_void_type(type)) {
- warning(expr->pos, "expression using sizeof(void)");
+ if (Wpointer_arith)
+ warning(expr->pos, "expression using sizeof(void)");
size = bits_in_char;
}
@@ -2204,7 +2205,8 @@ static struct symbol *evaluate_sizeof(struct expression
*expr)
}
if (is_function(type->ctype.base_type)) {
- warning(expr->pos, "expression using sizeof on a function");
+ if (Wpointer_arith)
+ warning(expr->pos, "expression using sizeof on a
function");
size = bits_in_char;
}
diff --git a/lib.c b/lib.c
index 73d372c36626538bab..f7fdac96674aec4c24 100644
--- a/lib.c
+++ b/lib.c
@@ -242,6 +242,7 @@ int Woverride_init = 1;
int Woverride_init_all = 0;
int Woverride_init_whole_range = 0;
int Wparen_string = 0;
+int Wpointer_arith = 0;
int Wptr_subtraction_blows = 0;
int Wreturn_void = 0;
int Wshadow = 0;
@@ -654,6 +655,7 @@ static const struct flag warnings[] = {
{ "return-void", &Wreturn_void },
{ "shadow", &Wshadow },
{ "sizeof-bool", &Wsizeof_bool },
+ { "pointer-arith", &Wpointer_arith },
{ "sparse-error", &Wsparse_error },
{ "tautological-compare", &Wtautological_compare },
{ "transparent-union", &Wtransparent_union },
diff --git a/lib.h b/lib.h
index 3050b5577ba4d42e97..e34bb9a02ebac03f52 100644
--- a/lib.h
+++ b/lib.h
@@ -151,6 +151,7 @@ extern int Woverride_init;
extern int Woverride_init_all;
extern int Woverride_init_whole_range;
extern int Wparen_string;
+extern int Wpointer_arith;
extern int Wptr_subtraction_blows;
extern int Wreturn_void;
extern int Wshadow;
diff --git a/sparse.1 b/sparse.1
index 88343f3170f195297a..4e1f2385fea7ec9296 100644
--- a/sparse.1
+++ b/sparse.1
@@ -288,6 +288,19 @@ Standard C syntax does not permit a parenthesized string
as an array
initializer. GCC allows this syntax as an extension. With
\fB\-Wparen\-string\fR, Sparse will warn about this syntax.
+Sparse does not issue these warnings by default.
+.
+.TP
+.B \-Wpointer\-arith
+Warn about anything that depends on the \fBsizeof\fR a void or function type.
+
+C99 does not allow the \fBsizeof\fR operator to be applied to function types
+or to incomplete types such as void. GCC allows \fBsizeof\fR to be applied to
+these types as an extension and assigns these types a size of \fI1\fR. With
+\fB\-pointer\-arith\fR, Sparse will warn about pointer arithmetic on void
+or function pointers, as well as expressions which directly apply the
+\fBsizeof\fR operator to void or function types.
+
Sparse does not issue these warnings by default.
.
.TP
diff --git a/validation/sizeof-function.c b/validation/sizeof-function.c
index 27d535d4ebda79ef0c..8ff67f2149981de7a9 100644
--- a/validation/sizeof-function.c
+++ b/validation/sizeof-function.c
@@ -35,7 +35,7 @@ int test(void)
/*
* check-name: sizeof-function
- * check-command: sparse -Wno-decl $file
+ * check-command: sparse -Wpointer-arith -Wno-decl $file
*
Re: [PATCH v3] sparse: add -Wpointer-arith flag to toggle sizeof(void) warnings
On Tue, Apr 10, 2018 at 09:41:19PM +0200, Luc Van Oostenryck wrote: > Thank you very much. > > There is just a problem with the test but it's my fault as I > pointed to you to my tree but the master tree lack a lot of fixes > and have problems when dereferencing function pointers. > So, for the master tree, I propose to use a version with these > tests removed: > git://github.com/lucvoo/sparse-dev.git pointer-arith-v3 Not a problem; I will rebase it on the v0.5.2-rc1 tag and take out the function pointer test-case. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH v3] sparse: add -Wpointer-arith flag to toggle sizeof(void) warnings
On Tue, Apr 10, 2018 at 12:09:44PM -1000, Joey Pabalinas wrote: > Not a problem; I will rebase it on the v0.5.2-rc1 tag and > take out the function pointer test-case. Sorry, I meant v0.5.2, heh. Sending the v4 now. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
[PATCH v4] sparse: add -Wpointer-arith flag to toggle sizeof(void) warnings
Recent changes to the min()/max() macros in include/linux/kernel.h
have added a lot of noise when compiling the kernel with Sparse checking
enabled. This mostly is due to the *huge* increase in the number of
sizeof(void) warnings, a larger number of which can safely be ignored.
Add the -Wpointer-arith flag to enable/disable these warnings (along
with the warning when applying sizeof to function types as well as
warning about pointer arithmetic on these types exactly like the
GCC -Wpointer-arith flag) on demand; the warning itself has been disabled
by default to reduce the large influx of noise which was inadvertently
added by commit 3c8ba0d61d04ced9f8 (kernel.h: Retain constant expression
output for max()/min()).
Update the manpage to document the new flag and add a validation case
for sizeof(void).
CC: Kees Cook
CC: Linus Torvalds
CC: Martin Uecker
CC: Al Viro
CC: Christopher Li
CC: Joey Pabalinas
CC: Luc Van Oostenryck
Signed-off-by: Joey Pabalinas
Signed-off-by: Luc Van Oostenryck
5 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/evaluate.c b/evaluate.c
index b96696d3a51396800a..8f07d08cf5b494f8f0 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2169,7 +2169,8 @@ static struct symbol *evaluate_sizeof(struct expression
*expr)
size = type->bit_size;
if (size < 0 && is_void_type(type)) {
- warning(expr->pos, "expression using sizeof(void)");
+ if (Wpointer_arith)
+ warning(expr->pos, "expression using sizeof(void)");
size = bits_in_char;
}
@@ -2180,7 +2181,8 @@ static struct symbol *evaluate_sizeof(struct expression
*expr)
}
if (is_function(type->ctype.base_type)) {
- warning(expr->pos, "expression using sizeof on a function");
+ if (Wpointer_arith)
+ warning(expr->pos, "expression using sizeof on a
function");
size = bits_in_char;
}
diff --git a/lib.c b/lib.c
index 645132a892107512a1..0caee38a996cd0fa4c 100644
--- a/lib.c
+++ b/lib.c
@@ -241,6 +241,7 @@ int Woverride_init = 1;
int Woverride_init_all = 0;
int Woverride_init_whole_range = 0;
int Wparen_string = 0;
+int Wpointer_arith = 0;
int Wptr_subtraction_blows = 0;
int Wreturn_void = 0;
int Wshadow = 0;
@@ -536,6 +537,7 @@ static const struct warning {
{ "return-void", &Wreturn_void },
{ "shadow", &Wshadow },
{ "sizeof-bool", &Wsizeof_bool },
+ { "pointer-arith", &Wpointer_arith },
{ "sparse-error", &Wsparse_error },
{ "tautological-compare", &Wtautological_compare },
{ "transparent-union", &Wtransparent_union },
diff --git a/lib.h b/lib.h
index a9b70b07686801305c..98e20d3830d9059acb 100644
--- a/lib.h
+++ b/lib.h
@@ -134,6 +134,7 @@ extern int Woverride_init;
extern int Woverride_init_all;
extern int Woverride_init_whole_range;
extern int Wparen_string;
+extern int Wpointer_arith;
extern int Wptr_subtraction_blows;
extern int Wreturn_void;
extern int Wshadow;
diff --git a/sparse.1 b/sparse.1
index e183204de623efd022..3bd5f1cb11309e65b8 100644
--- a/sparse.1
+++ b/sparse.1
@@ -282,6 +282,19 @@ Standard C syntax does not permit a parenthesized string
as an array
initializer. GCC allows this syntax as an extension. With
\fB\-Wparen\-string\fR, Sparse will warn about this syntax.
+Sparse does not issue these warnings by default.
+.
+.TP
+.B \-Wpointer\-arith
+Warn about anything that depends on the \fBsizeof\fR a void or function type.
+
+C99 does not allow the \fBsizeof\fR operator to be applied to function types
+or to incomplete types such as void. GCC allows \fBsizeof\fR to be applied to
+these types as an extension and assigns these types a size of \fI1\fR. With
+\fB\-pointer\-arith\fR, Sparse will warn about pointer arithmetic on void
+or function pointers, as well as expressions which directly apply the
+\fBsizeof\fR operator to void or function types.
+
Sparse does not issue these warnings by default.
.
.TP
diff --git a/validation/sizeof-void.c b/validation/sizeof-void.c
new file mode 100644
index 00..0fd917a21fb296a95d
--- /dev/null
+++ b/validation/sizeof-void.c
@@ -0,0 +1,44 @@
+#define is_constexpr(x) \
+ (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
+
+static int test(void)
+{
+ unsigned int s = 0, i = 0;
+ void *ptr = &i;
+
+ // OK
+ s += sizeof i;
+ s += sizeof &i;
+ s += sizeof ptr;
+ s += sizeof &ptr;
+
+ // KO
+ s += sizeof(void);
+ s += sizeof *ptr;
+ s += is_constexpr(ptr++);
+ s += is_constexpr((i++, 1));
+ s += is_constexpr(sizeof *ptr);
+ s += is_constexpr(ptr + 1);
+ s += is_constexpr(&ptr + 1);
+ s += is_constexpr(*(((c
Re: Code of Conduct: Let's revamp it.
On Wed, Sep 26, 2018 at 02:34:07PM -0500, \0xDynamite wrote: > >> You confuse the issue. My definition included "intended for the > >> public". But it isn't clear that open source code is intended for the > >> public -- it is intended for those who code or wish to. > > > Well, then I have to repeat myself: Signed-off source code (in form of > > patches) in a well-known programming language for a (nowadays) > > well-known GPLv2 licensed project mailed on "everyone can subscribe" > > mailinglists, (thus) to be found in several $SEARCH_ENGINE-indexed > > mailinglist archives, if accepted to be found in lots of publicly > > accessible git repos can be not intended to be published? > > You did it again. You changed words. I said intended for the public, > and you ended your sentence with "intended to be published". > > Like it or not, both the law and English grammar have ambiguities. > People put up with them because they share a common intuition (in a > lot of cases) of what each other means. > > If you share a birthday card with your personal love note inscribed > and the birthday girl sends it around to everyone at the party, have > you been violated? She might argue: how did you expect it to remain > private if you knew there were several people invited to the birthday > party? English does have oddities, agreed. However, open source code is definitely intended for the public as well. If I post an ad targeted at dog owners in my local town hall, it doesn't mean it's not intended for the public. Even though it is only for dog owners (or those who wish to be), it is still available freely to the general public. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message
On Sun, Nov 25, 2018 at 02:22:27AM +0100, Elvira Khabirova wrote: > Define two constants, PTRACE_EVENTMSG_SYSCALL_ENTRY and > PTRACE_EVENTMSG_SYSCALL_EXIT, and place them in ptrace_message > for the duration of syscall-stops. > This way ptracers can distinguish syscall-enter-stops > from syscall-exit-stops using PTRACE_GETEVENTMSG request. Is there an advantage to using two constants instead of a single sys_exit bit (set/unset for syscall-exit-stop/syscall-enter-stop)? -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message
On Sun, Nov 25, 2018 at 05:10:59AM +0300, Dmitry V. Levin wrote: > Given that without this patch the value returned by PTRACE_GETEVENTMSG > during syscall stop is undefined, we need two different ptrace_message > values that cannot be set by other ptrace events to enable reliable > identification of syscall-enter-stop and syscall-exit-stop in userspace: > if we make PTRACE_GETEVENTMSG return 0 or any other value routinely set by > other ptrace events, it would be hard for userspace to find out whether > the kernel implements new semantics or not. Ah, I see, I agree that definitely makes sense. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH v2 5/7] zram: support idle/huge page writeback
On Mon, Nov 26, 2018 at 05:28:11PM +0900, Minchan Kim wrote: > + strlcpy(mode_buf, buf, sizeof(mode_buf)); > + /* ignore trailing newline */ > + sz = strlen(mode_buf); One possible idea would be to use strscpy() instead and directly assign the return value to sz, avoiding an extra strlen() call (though you would have to check if `sz == -E2BIG` and do `sz = sizeof(mode_buf) - 1` in that case). > + if (!strcmp(mode_buf, "idle")) > + mode = IDLE_WRITEBACK; > + if (!strcmp(mode_buf, "huge")) > + mode = HUGE_WRITEBACK; Maybe using `else if (!strcmp(mode_buf, "huge"))` would be slightly better here, avoiding a second strcmp() if mode_buf has already matched "idle". > + if ((mode & IDLE_WRITEBACK && > + !zram_test_flag(zram, index, ZRAM_IDLE)) && > + (mode & HUGE_WRITEBACK && > + !zram_test_flag(zram, index, ZRAM_HUGE))) > + goto next; Wouldn't writing this as `mode & (IDLE_WRITEBACK | HUGE_WRITEBACK)` be a bit easier to read as well as slightly more compact? > + ret = len; > + __free_page(page); > +release_init_lock: > + up_read(&zram->init_lock); > + return ret; Hm, I noticed that this function either returns an error or just the passed in len on success, and I'm left wondering if there might be other useful information which could be passed back to the caller instead. I can't immediately think of any such information, though, so it's possible I'm just daydreaming :) -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [RFC][PATCH 08/14] function_graph: Remove unused task_curr_ret_stack()
On Wed, Nov 21, 2018 at 08:27:16PM -0500, Steven Rostedt wrote: > The static inline function task_curr_ret_stack() is unused, remove it. Just want ot make sure I understand this correctly, instead of using this function, the convention now is to just directly assign `t->curr_ret_stack = -1`? -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH] Little memset_explicit optimisation
On Sat, Nov 24, 2018 at 12:35:43PM +, David CARLIER wrote:
> Using the return value of memset for save/load sake.
>
> Signed-off-by: David Carlier
> ---
> lib/string.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/string.c b/lib/string.c
> index 38e4ca08e757..92da04a0213b 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -720,7 +720,7 @@ EXPORT_SYMBOL(memset);
> */
> void memzero_explicit(void *s, size_t count)
> {
> - memset(s, 0, count);
> + s = memset(s, 0, count);
> barrier_data(s);
> }
> EXPORT_SYMBOL(memzero_explicit);
Could you elaborate on the optimization that this patch performs?
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH v2 5/7] zram: support idle/huge page writeback
On Sun, Nov 25, 2018 at 11:47:37PM -1000, Joey Pabalinas wrote: > > + if ((mode & IDLE_WRITEBACK && > > + !zram_test_flag(zram, index, ZRAM_IDLE)) && > > + (mode & HUGE_WRITEBACK && > > + !zram_test_flag(zram, index, ZRAM_HUGE))) > > + goto next; > > Wouldn't writing this as `mode & (IDLE_WRITEBACK | HUGE_WRITEBACK)` > be a bit easier to read as well as slightly more compact? Scratch this comment, it just dawned on me that this is not an equivalent expression, heh. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: Re: [PATCH] Little memset_explicit optimisation
On Mon, Nov 26, 2018 at 07:36:19PM +, David CARLIER wrote: > Sorry I m not used yet at all to LKML rules. > > So here a slight difference in assembly generated between the two > versions (amd64) : > ` > .loc 1 7 7 > leaq-12(%rbp), %rax > movq%rax, -8(%rbp) > -.loc 1 11 2 > +.loc 1 9 6 > movq-8(%rbp), %rax > movl$4, %edx > movl$0, %esi > movq%rax, %rdi > callmemset@PLT > +movq%rax, -8(%rbp) > .loc 1 13 23 > movq-8(%rbp), %rax > movl(%rax), %eax What is the advantage of having the added `movq %rax, -8(%rbp)` here? The next instruction is `movq -8(%rbp), %rax` and nothing afterwords uses the value stored in `-8(%rbp)`. Also, is this compiled without optimization? Take a looks at the assembly in a small test case with -O1 (making sure to use the target variable so it isn't optimized out) and compare the assembly generated with and without that assignment. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: Re: [RFC][PATCH 08/14] function_graph: Remove unused task_curr_ret_stack()
On Mon, Nov 26, 2018 at 04:27:50PM -0500, Steven Rostedt wrote: > On Mon, 26 Nov 2018 00:02:07 -1000 > Joey Pabalinas wrote: > > > On Wed, Nov 21, 2018 at 08:27:16PM -0500, Steven Rostedt wrote: > > > The static inline function task_curr_ret_stack() is unused, remove it. > > > > Just want ot make sure I understand this correctly, instead of using this > > function, the convention now is to just directly assign `t->curr_ret_stack > > = -1`? > > > > Not sure what you mean. This function would just return the value of > curr_ret_stack, it didn't modify it. > > -- Steve Sorry for wording it a bit weird, I was mostly trying to figure out what these functions were originally used for. The uses I found in the git log were mostly things like: ret = trace_vprintk(ip, task_curr_ret_stack(current), fmt, ap); I guess the 2nd argument for these trace functions are implicit now and these functions are no longer needed anywhere? -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH v2 5/7] zram: support idle/huge page writeback
On Tue, Nov 27, 2018 at 11:13:27AM +0900, Minchan Kim wrote: > On Sun, Nov 25, 2018 at 11:47:37PM -1000, Joey Pabalinas wrote: > > On Mon, Nov 26, 2018 at 05:28:11PM +0900, Minchan Kim wrote: > > > + strlcpy(mode_buf, buf, sizeof(mode_buf)); > > > + /* ignore trailing newline */ > > > + sz = strlen(mode_buf); > > > > One possible idea would be to use strscpy() instead and directly assign > > the return value to sz, avoiding an extra strlen() call (though you would > > have to check if `sz == -E2BIG` and do `sz = sizeof(mode_buf) - 1` in that > > case). > > Thanks for the suggstion. > If I limit destination buffer smaller, I couldn't meet -E2BIG? -E2BIG return from strscpy() just means the string was longer than would fit in the length passed as the third argument. This means only `sizeof(mode_buf) - 1` bytes were copied into the dest, with the last byte being the \0 terminator. You can find the function source in lib/string.c. > > > + ret = len; > > > + __free_page(page); > > > +release_init_lock: > > > + up_read(&zram->init_lock); > > > + return ret; > > > > Hm, I noticed that this function either returns an error or just the passed > > in len on success, and I'm left wondering if there might be other useful > > information which could be passed back to the caller instead. I can't > > immediately think of any such information, though, so it's possible I'm > > just daydreaming :) > > It is write syscall semantic of sysfs so not sure it's doable to pass > other value to user. Well, with the write system call you can have partial writes, in which case it would return the (short) number of bytes written. But in this function, even if there was some sort of partial write possible, this function still only every returns len or error. Neither of these are that important though, so Ack from me with or without these two suggestions. Reviewed-by: Joey Pabalinas -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCHi v2] mm: put_and_wait_on_page_locked() while page is migrated
On Tue, Nov 27, 2018 at 01:08:50PM -0800, Hugh Dickins wrote:
> On Tue, 27 Nov 2018, Mike Rapoport wrote:
> > On Mon, Nov 26, 2018 at 11:27:07AM -0800, Hugh Dickins wrote:
> > >
> > > +/*
> > > + * A choice of three behaviors for wait_on_page_bit_common():
> > > + */
> > > +enum behavior {
> > > + EXCLUSIVE, /* Hold ref to page and take the bit when woken, like
> > > + * __lock_page() waiting on then setting PG_locked.
> > > + */
> > > + SHARED, /* Hold ref to page and check the bit when woken, like
> > > + * wait_on_page_writeback() waiting on PG_writeback.
> > > + */
> > > + DROP, /* Drop ref to page before wait, no check when woken,
> > > + * like put_and_wait_on_page_locked() on PG_locked.
> > > + */
> > > +};
> >
> > Can we please make it:
> >
> > /**
> > * enum behavior - a choice of three behaviors for wait_on_page_bit_common()
> > */
> > enum behavior {
> > /**
> > * @EXCLUSIVE: Hold ref to page and take the bit when woken,
> > * like __lock_page() waiting on then setting %PG_locked.
> > */
> > EXCLUSIVE,
> > /**
> > * @SHARED: Hold ref to page and check the bit when woken,
> > * like wait_on_page_writeback() waiting on %PG_writeback.
> > */
> > SHARED,
> > /**
> > * @DROP: Drop ref to page before wait, no check when woken,
> > * like put_and_wait_on_page_locked() on %PG_locked.
> > */
> > DROP,
> > };
>
> I'm with Matthew, I'd prefer not: the first looks a more readable,
> less cluttered comment to me than the second: this is just an arg
> to an internal helper in mm/filemap.c, itself not kernel-doc'ed.
>
> But the comment is not there for me: if consensus is that the
> second is preferable, then sure, we can change it over.
For something which is internal to a single file I strongly prefer
the first as well.
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH] selftests: add TPM 2.0 tests
On Tue, Nov 27, 2018 at 02:10:48PM -0800, Jarkko Sakkinen wrote:
> Added the tests that I've been using for testing TPM 2.0 functionality
> for long time but have out-of-tree so far residing in
>
> https://github.com/jsakkine-intel/tpm2-scripts
>
> Cc: Tadeusz Struk
> Signed-off-by: Jarkko Sakkinen
Just one thing I didn't really understand:
> +def start_auth_session(self, session_type, name_alg = TPM2_ALG_SHA1):
> +fmt = '>HII IIH16sHBHH'
> +cmd = struct.pack(fmt,
> + TPM2_ST_NO_SESSIONS,
> + struct.calcsize(fmt),
> + TPM2_CC_START_AUTH_SESSION,
> + TPM2_RH_NULL,
> + TPM2_RH_NULL,
> + 16,
> + '\0' * 16,
> + 0,
> + session_type,
> + TPM2_ALG_NULL,
> + name_alg)
> +
> +return struct.unpack('>I', self.send_cmd(cmd)[10:14])[0]
> +
> +def __calc_pcr_digest(self, pcrs, bank_alg = TPM2_ALG_SHA1,
> + digest_alg = TPM2_ALG_SHA1):
> +x = []
Is there a reason for using `'\0' * 16` there instead of just 0?
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH] selftests: add TPM 2.0 tests
On Tue, Nov 27, 2018 at 12:49:00PM -1000, Joey Pabalinas wrote:
> > +def start_auth_session(self, session_type, name_alg = TPM2_ALG_SHA1):
> > +fmt = '>HII IIH16sHBHH'
> > +cmd = struct.pack(fmt,
> > + TPM2_ST_NO_SESSIONS,
> > + struct.calcsize(fmt),
> > + TPM2_CC_START_AUTH_SESSION,
> > + TPM2_RH_NULL,
> > + TPM2_RH_NULL,
> > + 16,
> > + '\0' * 16,
> > + 0,
> > + session_type,
> > + TPM2_ALG_NULL,
> > + name_alg)
> > +
> > +return struct.unpack('>I', self.send_cmd(cmd)[10:14])[0]
> > +
> > +def __calc_pcr_digest(self, pcrs, bank_alg = TPM2_ALG_SHA1,
> > + digest_alg = TPM2_ALG_SHA1):
> > +x = []
>
> Is there a reason for using `'\0' * 16` there instead of just 0?
Nevermind, I keep forgetting that the * operator on strings performs
repetition. I've been programming too much C lately, it seems.
Acked-By: Joey Pabalinas
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH] Little memset_explicit optimisation
On Wed, Nov 28, 2018 at 06:32:27AM +, David CARLIER wrote: > Bad entrance with bad idea I m afraid :-) sorry for the noise. We all start somewhere, no worries :) -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH v1 2/2] signal: add procfd_signal() syscall
On Mon, Nov 19, 2018 at 11:32:39AM +0100, Christian Brauner wrote:
> + if (info) {
> + ret = __copy_siginfo_from_user(sig, &kinfo, info);
> + if (unlikely(ret))
> + goto err;
What's the reason you don't propagate up the errors from
__copy_siginfo_from_user()?
Granted, I admit that -E2BIG is kind of weird to return, but -EFAULT seems like
a
fairly sane error.
Or is there some reason it's more useful to just return -EINVAL for all of the
failure cases here?
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH v1 2/2] signal: add procfd_signal() syscall
On Wed, Nov 28, 2018 at 11:05:49PM +0100, Christian Brauner wrote:
> On Wed, Nov 28, 2018 at 11:45:34AM -1000, Joey Pabalinas wrote:
> > On Mon, Nov 19, 2018 at 11:32:39AM +0100, Christian Brauner wrote:
> > > + if (info) {
> > > + ret = __copy_siginfo_from_user(sig, &kinfo, info);
> > > + if (unlikely(ret))
> > > + goto err;
> >
>
> I think you're misreading here. It jumps to:
>
> err:
> fdput(f);
> return ret;
>
> and does propagate the error. This is also an old iteration of the patch.
Whoops, that I am. Sorry about that.
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH 26/29] x86/fpu: Inline copy_user_to_fpregs_zeroing()
On Wed, Nov 28, 2018 at 11:20:32PM +0100, Sebastian Andrzej Siewior wrote: > Start refactoring __fpu__restore_sig() by inlining > copy_user_to_fpregs_zeroing(). > > Signed-off-by: Sebastian Andrzej Siewior Reviewed-by: Joey Pabalinas -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH] binder: remove BINDER_DEBUG_ENTRY()
On Fri, Nov 30, 2018 at 08:26:30PM -0500, Yangtao Li wrote:
> We already have the DEFINE_SHOW_ATTRIBUTE.There is no need to define
> such a macro,so remove BINDER_DEBUG_ENTRY.
>
> Signed-off-by: Yangtao Li
Good catch.
Reviewed-by: Joey Pabalinas
On Fri, Nov 30, 2018 at 08:26:30PM -0500, Yangtao Li wrote:
> We already have the DEFINE_SHOW_ATTRIBUTE.There is no need to define
> such a macro,so remove BINDER_DEBUG_ENTRY.
>
> Signed-off-by: Yangtao Li
> ---
> drivers/android/binder.c | 48 ++--
> 1 file changed, 17 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index cb30a524d16d..5496b8e07234 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -94,22 +94,8 @@ static struct dentry *binder_debugfs_dir_entry_root;
> static struct dentry *binder_debugfs_dir_entry_proc;
> static atomic_t binder_last_id;
>
> -#define BINDER_DEBUG_ENTRY(name) \
> -static int binder_##name##_open(struct inode *inode, struct file *file) \
> -{ \
> - return single_open(file, binder_##name##_show, inode->i_private); \
> -} \
> -\
> -static const struct file_operations binder_##name##_fops = { \
> - .owner = THIS_MODULE, \
> - .open = binder_##name##_open, \
> - .read = seq_read, \
> - .llseek = seq_lseek, \
> - .release = single_release, \
> -}
> -
> -static int binder_proc_show(struct seq_file *m, void *unused);
> -BINDER_DEBUG_ENTRY(proc);
> +static int proc_show(struct seq_file *m, void *unused);
> +DEFINE_SHOW_ATTRIBUTE(proc);
>
> /* This is only defined in include/asm-arm/sizes.h */
> #ifndef SZ_1K
> @@ -4964,7 +4950,7 @@ static int binder_open(struct inode *nodp, struct file
> *filp)
> proc->debugfs_entry = debugfs_create_file(strbuf, 0444,
> binder_debugfs_dir_entry_proc,
> (void *)(unsigned long)proc->pid,
> - &binder_proc_fops);
> + &proc_fops);
> }
>
> return 0;
> @@ -5592,7 +5578,7 @@ static void print_binder_proc_stats(struct seq_file *m,
> }
>
>
> -static int binder_state_show(struct seq_file *m, void *unused)
> +static int state_show(struct seq_file *m, void *unused)
> {
> struct binder_proc *proc;
> struct binder_node *node;
> @@ -5631,7 +5617,7 @@ static int binder_state_show(struct seq_file *m, void
> *unused)
> return 0;
> }
>
> -static int binder_stats_show(struct seq_file *m, void *unused)
> +static int stats_show(struct seq_file *m, void *unused)
> {
> struct binder_proc *proc;
>
> @@ -5647,7 +5633,7 @@ static int binder_stats_show(struct seq_file *m, void
> *unused)
> return 0;
> }
>
> -static int binder_transactions_show(struct seq_file *m, void *unused)
> +static int transactions_show(struct seq_file *m, void *unused)
> {
> struct binder_proc *proc;
>
> @@ -5660,7 +5646,7 @@ static int binder_transactions_show(struct seq_file *m,
> void *unused)
> return 0;
> }
>
> -static int binder_proc_show(struct seq_file *m, void *unused)
> +static int proc_show(struct seq_file *m, void *unused)
> {
> struct binder_proc *itr;
> int pid = (unsigned long)m->private;
> @@ -5703,7 +5689,7 @@ static void print_binder_transaction_log_entry(struct
> seq_file *m,
> "\n" : " (incomplete)\n");
> }
>
> -static int binder_transaction_log_show(struct seq_file *m, void *unused)
> +static int transaction_log_show(struct seq_file *m, void *unused)
> {
> struct binder_transaction_log *log = m->private;
> unsigned int log_cur = atomic_read(&log->cur);
> @@ -5735,10 +5721,10 @@ static const struct file_operations binder_fops = {
> .release = binder_release,
> };
>
> -BINDER_DEBUG_ENTRY(state);
> -BINDER_DEBUG_ENTRY(stats);
> -BINDER_DEBUG_ENTRY(transactions);
> -BINDER_DEBUG_ENTRY(transaction_log);
> +DEFINE_SHOW_ATTRIBUTE(state);
> +DEFINE_SHOW_ATTRIBUTE(stats);
> +DEFINE_SHOW_ATTRIBUTE(transactions);
> +DEFINE_SHOW_ATTRIBUTE(transaction_log);
>
> static int __init init_binder_device(const char *name)
> {
> @@ -5792,27 +5778,27 @@ static int __init binder_init(void)
> 0444,
> binder_debugfs_dir_entry_root,
> NULL,
> - &binder_state_fops);
> + &state_fops);
> debugfs_create_file("stats",
>
Re: [PATCH] sched/fair: Fix assignment of boolean variables
On Sat, Dec 01, 2018 at 05:09:36PM +0800, Wen Yang wrote:
> Fix the following warnings reported by coccinelle:
> kernel//sched/fair.c:7958:3-12: WARNING: Assignment of bool to 0/1
>
> This also makes the code more readable.
>
> Signed-off-by: Wen Yang
> CC: Ingo Molnar
> CC: Peter Zijlstra
> CC: linux-kernel@vger.kernel.org
Ack, earlier assignments in the function like:
if (nr_running > 1)
*overload = true;
use `= true`, so this change keeps things consistent.
Reviewed-by: Joey Pabalinas
> ---
> kernel/sched/fair.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index a2e8160968cb..f19aa66f9b15 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7955,7 +7955,7 @@ static inline void update_sg_lb_stats(struct lb_env
> *env,
> if (env->sd->flags & SD_ASYM_CPUCAPACITY &&
> sgs->group_misfit_task_load < rq->misfit_task_load) {
> sgs->group_misfit_task_load = rq->misfit_task_load;
> - *overload = 1;
> + *overload = true;
> }
> }
>
> --
> 2.19.1
>
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH] drop silly "static inline asmlinkage" from dump_stack()
On Sat, Nov 24, 2018 at 12:35:30PM +0300, Alexey Dobriyan wrote:
> -static inline asmlinkage void dump_stack(void)
> +static inline void dump_stack(void)
Why is it "silly"? An explanation in the commit message would be useful.
> Signed-off-by: Alexey Dobriyan
> ---
>
> include/linux/printk.h |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -269,7 +269,7 @@ static inline void show_regs_print_info(const char
> *log_lvl)
> {
> }
>
> -static inline asmlinkage void dump_stack(void)
> +static inline void dump_stack(void)
> {
> }
>
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH] drop silly "static inline asmlinkage" from dump_stack()
On Sun, Dec 02, 2018 at 12:02:54PM +0300, Alexey Dobriyan wrote: > Empty function will be inlined so asmlinkage doesn't do anything. Yes, that is an example of a perfect explanation to have in the commit message :) Ack from me after that addition. Acked-by: Joey Pabalinas -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH v3 0/7] zram idle page writeback
On Tue, Nov 27, 2018 at 02:54:22PM +0900, Minchan Kim wrote: > Inherently, swap device has many idle pages which are rare touched since > it was allocated. It is never problem if we use storage device as swap. > However, it's just waste for zram-swap. > > This patchset supports zram idle page writeback feature. Revisions look good to me. Will also try to give it some testing this week. Reviewed-by: Joey Pabalinas > * Admin can define what is idle page "no access since X time ago" > * Admin can define when zram should writeback them > * Admin can define when zram should stop writeback to prevent wearout > > Detail is on each patch's description. > > Below first two patches are -stable material so it could go first > separately with others in this series. > > zram: fix lockdep warning of free block handling > zram: fix double free backing device > > * from v2 > - use strscpy instead of strlcpy - Joey Pabalinas > - remove irqlock for bitmap op - akpm > - don't use page as stat unit - akpm > > * from v1 > - add fix dobule free backing device - minchan > - change writeback/idle interface - minchan > - remove direct incompressible page writeback - sergey > > Minchan Kim (7): > zram: fix lockdep warning of free block handling > zram: fix double free backing device > zram: refactoring flags and writeback stuff > zram: introduce ZRAM_IDLE flag > zram: support idle/huge page writeback > zram: add bd_stat statistics > zram: writeback throttle > > Documentation/ABI/testing/sysfs-block-zram | 32 ++ > Documentation/blockdev/zram.txt| 51 ++- > drivers/block/zram/Kconfig | 5 +- > drivers/block/zram/zram_drv.c | 501 +++-- > drivers/block/zram/zram_drv.h | 19 +- > 5 files changed, 446 insertions(+), 162 deletions(-) > > -- > 2.20.0.rc0.387.gc7a69e6b6c-goog > -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
[PATCH 2/4] HID: multitouch: don't check HID_GROUP_MULTITOUCH_WIN_8 for serial protocol
The HID_GROUP_MULTITOUCH_WIN_8 group never needs to check for the serial
protocol, so avoid setting `td->serial_maybe = true;` in order to avoid
an unnecessary mt_post_parse_default_settings() call
Signed-off-by: Joey Pabalinas
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index a793076139d7d0db9b..c0654db0b736543ca0 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1460,11 +1460,13 @@ static int mt_probe(struct hid_device *hdev, const
struct hid_device_id *id)
if (!td->fields) {
dev_err(&hdev->dev, "cannot allocate multitouch fields data\n");
return -ENOMEM;
}
- if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
+ if (id->vendor == HID_ANY_ID
+ && id->product == HID_ANY_ID
+ && id->group != HID_GROUP_MULTITOUCH_WIN_8)
td->serial_maybe = true;
/* This allows the driver to correctly support devices
* that emit events over several HID messages.
*/
--
2.18.0
[PATCH 4/4] HID: multitouch: remove unneeded else conditional cases
Elide lone `else` cases and replace `else if` clauses
with plain `if` conditionals when they occur immediately
after return statements.
Signed-off-by: Joey Pabalinas
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 08b50e5908cecdda66..30b1a2c67f39a41325 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -205,12 +205,12 @@ static void mt_post_parse(struct mt_device *td);
static int cypress_compute_slot(struct mt_device *td)
{
if (td->curdata.contactid != 0 || td->num_received == 0)
return td->curdata.contactid;
- else
- return -1;
+
+ return -1;
}
static struct mt_class mt_classes[] = {
{ .name = MT_CLS_DEFAULT,
.quirks = MT_QUIRK_ALWAYS_VALID |
@@ -803,12 +803,12 @@ static int mt_compute_timestamp(struct mt_device *td,
struct hid_field *field,
delta *= 100;
if (jdelta > MAX_TIMESTAMP_INTERVAL)
/* No data received for a while, resync the timestamp. */
return 0;
- else
- return td->timestamp + delta;
+
+ return td->timestamp + delta;
}
static int mt_touch_event(struct hid_device *hid, struct hid_field *field,
struct hid_usage *usage, __s32 value)
{
@@ -1110,11 +1110,12 @@ static int mt_input_mapping(struct hid_device *hdev,
struct hid_input *hi,
* HID_DG_CONTACTCOUNT from the pen report as it is outside the physical
* collection, but within the report ID.
*/
if (field->physical == HID_DG_STYLUS)
return 0;
- else if ((field->physical == 0) &&
+
+ if ((field->physical == 0) &&
(field->report->id != td->mt_report_id) &&
(td->mt_report_id != -1))
return 0;
if (field->application == HID_DG_TOUCHSCREEN ||
--
2.18.0
[PATCH 0/4] reduce Surface Pro 3 multitouch jitter
The Surface Pro 3 firmware doesn't reliably send contact lift off reports nor handle invalid report values gracefully. To reduce touchscreen input jitter: - add MT_QUIRK_NOT_SEEN_MEANS_UP to the MT_CLS_WIN_8 - drop invalid report values Joey Pabalinas (4): HID: multitouch: add MT_QUIRK_NOT_SEEN_MEANS_UP to MT_CLS_WIN_8 quirks HID: multitouch: don't check HID_GROUP_MULTITOUCH_WIN_8 for serial protocol HID: multitouch: drop reports containing invalid values HID: multitouch: remove unneeded else conditional cases drivers/hid/hid-multitouch.c | 27 --- 1 file changed, 20 insertions(+), 7 deletions(-) -- 2.18.0
[PATCH 3/4] HID: multitouch: drop reports containing invalid values
Avoid processing reports containing invalid values to reduce
multitouch input stutter.
Signed-off-by: Joey Pabalinas
1 file changed, 9 insertions(+)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index c0654db0b736543ca0..08b50e5908cecdda66 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -694,10 +694,19 @@ static void mt_complete_slot(struct mt_device *td, struct
input_dev *input)
{
if ((td->mtclass.quirks & MT_QUIRK_CONTACT_CNT_ACCURATE) &&
td->num_received >= td->num_expected)
return;
+ /* drop invalid values after counting them */
+ if (td->curdata.x == 0x &&
+ td->curdata.y == 0x &&
+ td->curdata.w == 0x &&
+ td->curdata.h == 0x) {
+ td->num_received++;
+ return;
+ }
+
if (td->curvalid || (td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID)) {
int active;
int slotnum = mt_compute_slot(td, input);
struct mt_slot *s = &td->curdata;
struct input_mt *mt = input->mt;
--
2.18.0
[PATCH 1/4] HID: multitouch: add MT_QUIRK_NOT_SEEN_MEANS_UP to MT_CLS_WIN_8 quirks
The firmware found in the touch screen of the Surface Pro 3 is slightly
buggy and occasionally doesn't send lift off reports for contacts; add
MT_QUIRK_NOT_SEEN_MEANS_UP to .quirks to compensate for the missed
reports.
Signed-off-by: Joey Pabalinas
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 45968f7970f87775fa..a793076139d7d0db9b 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -242,11 +242,12 @@ static struct mt_class mt_classes[] = {
.quirks = MT_QUIRK_ALWAYS_VALID |
MT_QUIRK_IGNORE_DUPLICATES |
MT_QUIRK_HOVERING |
MT_QUIRK_CONTACT_CNT_ACCURATE |
MT_QUIRK_STICKY_FINGERS |
- MT_QUIRK_WIN8_PTP_BUTTONS },
+ MT_QUIRK_WIN8_PTP_BUTTONS |
+ MT_QUIRK_NOT_SEEN_MEANS_UP },
{ .name = MT_CLS_EXPORT_ALL_INPUTS,
.quirks = MT_QUIRK_ALWAYS_VALID |
MT_QUIRK_CONTACT_CNT_ACCURATE,
.export_all_inputs = true },
{ .name = MT_CLS_WIN_8_DUAL,
--
2.18.0
Re: [PATCH] scripts/tags.sh: don't rely on parsing `ls` for $ALLSOURCE_ARCHS generation
On Tue, May 15, 2018 at 02:13:11PM -1000, Joey Pabalinas wrote: > and directly assign it to $ALLSOURCE_ARCHS; use a subshell > so `cd` doesn't affect the current working directory. Whoops, turns out the inner `()` isn't needed, so going to revise and send a v2. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
[PATCH v2] scripts/tags.sh: don't parse `ls` for $ALLSOURCE_ARCHS generation
Parsing `ls` is fragile at best and _will_ fail when $tree
contains spaces. Replace this with a glob-generated string
and directly assign it to $ALLSOURCE_ARCHS; a subshell is
implied by $(), so `cd` doesn't affect the current working
directory.
Signed-off-by: Joey Pabalinas
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/scripts/tags.sh b/scripts/tags.sh
index 78e546ff689c2d5f40..a4b089aa35efbc0a13 100755
--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -29,14 +29,11 @@ fi
ignore="$ignore ( -path ${tree}tools ) -prune -o"
# Find all available archs
find_all_archs()
{
- ALLSOURCE_ARCHS=""
- for arch in `ls ${tree}arch`; do
- ALLSOURCE_ARCHS="${ALLSOURCE_ARCHS} "${arch##\/}
- done
+ ALLSOURCE_ARCHS="$(cd "${tree}arch/" && echo *)"
}
# Detect if ALLSOURCE_ARCHS is set. If not, we assume SRCARCH
if [ "${ALLSOURCE_ARCHS}" = "" ]; then
ALLSOURCE_ARCHS=${SRCARCH}
--
2.17.0.rc1.35.g90bbd502d54fe92035.dirty
signature.asc
Description: PGP signature
Re: [PATCH] scripts/tags.sh: don't rely on parsing `ls` for $ALLSOURCE_ARCHS generation
On Fri, May 18, 2018 at 02:46:32PM +0900, Masahiro Yamada wrote:
> Andrew picked it up, but this patch is *bad*
>
> You missed arch/Kconfig.
>
> $(cd "${tree}arch/" && echo *)
> contains Kconfig, but it is not arch.
That was also something that I found a bit weird myself, but I had
assumed there was a good reason for keeping that. The original
command also returns a string containing Kconfig:
> tree="$PWD/"
> echo "$tree"
> ALLSOURCE_ARCHS=""
> for arch in `ls ${tree}arch`; do
> ALLSOURCE_ARCHS="${ALLSOURCE_ARCHS} "${arch##\/}
> done
> echo "$ALLSOURCE_ARCHS"'
gives the same output as my command (albeit with an extra leading
space that shouldn't be important):
> /store/code/projects/kernel/linux/
> Kconfig alpha arc arm arm64 c6x h8300 hexagon ia64 m68k microblaze mips
> nds32 nios2 openrisc parisc powerpc riscv s390 sh sparc um unicore32 x86
> xtensa
However, if there really is no reason for that being there, I
have no complaints against fixing it. I'll send a v3 in a bit.
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
[PATCH v3] scripts/tags.sh: don't parse `ls` for $ALLSOURCE_ARCHS generation
Parsing `ls` is fragile at best and _will_ fail when $tree
contains spaces. Replace this with a glob-generated string
and directly assign it to $ALLSOURCE_ARCHS (Kconfig is
removed as it isn't an architecture); a subshell is implied
by $(), so `cd` doesn't affect the current working directory.
Signed-off-by: Joey Pabalinas
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/scripts/tags.sh b/scripts/tags.sh
index 78e546ff689c2d5f40..e4aba2983f6272fc44 100755
--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -29,14 +29,12 @@ fi
ignore="$ignore ( -path ${tree}tools ) -prune -o"
# Find all available archs
find_all_archs()
{
- ALLSOURCE_ARCHS=""
- for arch in `ls ${tree}arch`; do
- ALLSOURCE_ARCHS="${ALLSOURCE_ARCHS} "${arch##\/}
- done
+ ALLSOURCE_ARCHS="$(cd "${tree}arch/" && echo *)"
+ ALLSOURCE_ARCHS="${ALLSOURCE_ARCHS/Kconfig}"
}
# Detect if ALLSOURCE_ARCHS is set. If not, we assume SRCARCH
if [ "${ALLSOURCE_ARCHS}" = "" ]; then
ALLSOURCE_ARCHS=${SRCARCH}
--
2.17.0.rc1.35.g90bbd502d54fe92035.dirty
signature.asc
Description: PGP signature
Re: [PATCH v3] scripts/tags.sh: don't parse `ls` for $ALLSOURCE_ARCHS generation
On Fri, May 18, 2018 at 04:08:44PM +0900, Masahiro Yamada wrote:
> I do not like hard-coding the file(s) in arch/.
>
> Don't you have an idea to list only directories?
> 'find -type d' or 'ls -F' or something?
How do you feel about the following?
> # Detect if ALLSOURCE_ARCHS is set. If not, we assume SRCARCH
> if [ "${ALLSOURCE_ARCHS}" = "" ]; then
> ALLSOURCE_ARCHS="${SRCARCH}"
> elif [ "${ALLSOURCE_ARCHS}" = "all" ]; then
> ALLSOURCE_ARCHS="$(cd "${tree}arch/" && find . -maxdepth 1 -type d |
> paste -sd' ')"
> ALLSOURCE_ARCHS="${ALLSOURCE_ARCHS//[.\/]}"
> fi
If something like that if acceptable, I'll use that for the v4.
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH v3] scripts/tags.sh: don't parse `ls` for $ALLSOURCE_ARCHS generation
On Fri, May 18, 2018 at 06:52:48PM +0900, Masahiro Yamada wrote: > Why is 'paste' necessary? Without it newlines from find are retained from the find output (at least in bash when I tested it). Though it doesn't really matter here since we are feeding it to a for loop, I just wanted to be sure this didn't surprise someone in the future by squeezing the newlines into spaces. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH v3] scripts/tags.sh: don't parse `ls` for $ALLSOURCE_ARCHS generation
On Fri, May 18, 2018 at 08:03:26PM +0900, Masahiro Yamada wrote:
> Does this work?
> ALLSOURCE_ARCHS=$(find "${tree}"arch -mindepth 1 -maxdepth 1 -type d
> -printf ' %f')
Aha, that actually works perfectly.
> It would leave one leading space, but I do not think it is a big deal.
>
>
> It would be possible to chop it like follows, but I wonder if it is worth
> doing.
> ALLSOURCE_ARCHS=$(find "${tree}"arch -mindepth 1 -maxdepth 1 -type d
> -printf ' %f' | cut -c2-)
Yeah, the leading space isn't really something that is worth worrying
about here in my opinion either.
I'll use this in my v4 then, thanks.
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
[PATCH v4] scripts/tags.sh: use `find` for $ALLSOURCE_ARCHS generation
Parsing `ls` is fragile at best and _will_ fail when $tree
contains spaces. Replace this with a find-generated string
and directly assign it to $ALLSOURCE_ARCHS; a subshell is
implied by $(), so `cd` doesn't affect the current working
directory.
Signed-off-by: Joey Pabalinas
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/scripts/tags.sh b/scripts/tags.sh
index 78e546ff689c2d5f40..c08347fdeef12a7621 100755
--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -26,24 +26,15 @@ else
fi
# ignore userspace tools
ignore="$ignore ( -path ${tree}tools ) -prune -o"
-# Find all available archs
-find_all_archs()
-{
- ALLSOURCE_ARCHS=""
- for arch in `ls ${tree}arch`; do
- ALLSOURCE_ARCHS="${ALLSOURCE_ARCHS} "${arch##\/}
- done
-}
-
# Detect if ALLSOURCE_ARCHS is set. If not, we assume SRCARCH
if [ "${ALLSOURCE_ARCHS}" = "" ]; then
ALLSOURCE_ARCHS=${SRCARCH}
elif [ "${ALLSOURCE_ARCHS}" = "all" ]; then
- find_all_archs
+ ALLSOURCE_ARCHS="$(find "${tree}arch/" -mindepth 1 -maxdepth 1 -type d
-printf ' %f')"
fi
# find sources in arch/$ARCH
find_arch_sources()
{
--
2.17.0.rc1.35.g90bbd502d54fe92035.dirty
signature.asc
Description: PGP signature
Re: [RFC] LKML Archive in Maildir Format
On Tue, Dec 18, 2018 at 09:26:27PM +0100, Jasper Spaans wrote: > Now you've caught my attention; first of all, there are more than 3M > messages stored in the lkml.org datase, so I guess you've missed some > messages or something is really broken. > > Besides, unless you figured out how to get to the raw data, you've just > scraped a rendering which discards stuff like pgp signatures etc and has > very incomplete headers. Unless you don't care for those of course :) > > Note that I've also been toying with the lore dataset, and wrote a tiny tool > to get Maildir-like data out of it; this code is a bit of a single-use-jig > so you'll need to do some coding if you really want to use it. Attached > anyway. Yeah, after looking closer at it last week, something here is very weird. This is definitely far from complete. When I have some free time I'm just going to give it another go with the public-inbox conversion. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
[RFC] LKML Archive in Maildir Format
I spent a lot of time trying to find an LKML archive in Maildir format that I could use for local searches with nutmuch or something, but all the links I was able to find were all dead. I ended up just compiling one myself and I currently host it at: https://alyptik.org/lkml.tar.xz It's possible I'm the only weirdo who finds this kind of thing useful, but I figured I should share it just in case I'm not. It's about 1.1 million files, I was wondering if anyone had an idea of a better way to host this? I've tried Github and GitLab, but they don't appreciate repos with that many files, hah. Open to suggestions, thanks! -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [RFC] LKML Archive in Maildir Format
On Sun, Dec 16, 2018 at 11:17:34AM -0800, Joe Perches wrote: > On Sun, 2018-12-16 at 09:06 -1000, Joey Pabalinas wrote: > > I spent a lot of time trying to find an LKML archive in Maildir format > > that I could use for local searches with nutmuch or something, but all > > the links I was able to find were all dead. > > You might instead use > > https://www.kernel.org/lore.html > https://git.kernel.org/pub/scm/public-inbox/vger.kernel.org/git.git/ That was my first attempt, but the ducumentation for the public-inbox format is sort of terrible, and after a few hours trying to convert it to Maildir I just gave up. I ended up just slowly scraping lkml.org for a couple weeks so I wouldn't disrupt anything and it worked fairly well. Just looking for advice on where to host this now so others might be able to use it. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [RFC] LKML Archive in Maildir Format
On Sun, Dec 16, 2018 at 02:46:49PM -0500, Konstantin Ryabitsev wrote: > On Sun, Dec 16, 2018 at 09:06:39AM -1000, Joey Pabalinas wrote: > > I spent a lot of time trying to find an LKML archive in Maildir format > > that I could use for local searches with nutmuch or something, but all > > the links I was able to find were all dead. > > > > I ended up just compiling one myself and I currently host it at: > > > > https://alyptik.org/lkml.tar.xz > > You seem to have duplicated a lot of effort that has already been done > to compile the archive on lore.kernel.org. Absolutely correct, haha. > > > It's possible I'm the only weirdo who finds this kind of thing useful, but > > I figured I should share it just in case I'm not. > > The maildir format is kind of terrible for LKML, because having millions > of messages in a single directory is very hard on the underlying FS. If > you break it up into multiple folders, then it becomes difficult to > search. This is the main reason why we have chosen to go with the > public-inbox format, which solves both of these problems and allows for > a very efficient archive updating and replication using git. > > > It's about 1.1 million files, I was wondering if anyone had an idea of a > > better way to host this? I've tried Github and GitLab, but they don't > > appreciate repos with that many files, hah. > > Like I said, you seem to be going down the road we've already tried and > rejected. :) Yes, I had a strong suspicion I might be the only crazy person who prefers this kind of format :) My only comment on the public-mailbox choice is that the documentation is very sparse and erratic. Myself and a couple other people just couldn't figure out how to convert that format to Maildir or some other format you could feed into a reader like neomutt. Do you have any advice on how to convert those public-inbox files correctly? -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [RFC] LKML Archive in Maildir Format
On Sun, Dec 16, 2018 at 02:55:05PM -0500, Konstantin Ryabitsev wrote: > On Sun, Dec 16, 2018 at 09:21:35AM -1000, Joey Pabalinas wrote: > > That was my first attempt, but the ducumentation for the public-inbox > > format is sort of terrible, > > I'm surprised you think so, because it's basically a simple file called > "m" that is updated on each commit and contains the body of the > message. > > > and after a few hours trying to convert it to Maildir I just gave up. > > It's as easy as something like this: > > for commit in $(git rev-list master); do: > git show $commit:m > maildir/new/$commit > done > > You have to do it per each of the shards for the complete archive. Ah dang, I was trying to use stuff like ssoma to split it, no wonder it didn't work. Not sure why I didn't think to try any git commands... Well, at least now I know, ha. Thanks! -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH] Staging: nvec: nvec: fixed check style issues
On Sun, Dec 16, 2018 at 01:43:54PM -0800, Amir Mahdi Ghorbanian wrote:
> Replaced udelay() by the preferred usleep_range() function.
>
> Signed-off-by: Amir Mahdi Ghorbanian
Nack, usleep_range isn't for atomic contexts like interrupt handlers.
> ---
> drivers/staging/nvec/nvec.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> index 08027a3..6f35f92 100644
> --- a/drivers/staging/nvec/nvec.c
> +++ b/drivers/staging/nvec/nvec.c
> @@ -626,7 +626,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
> break;
> case 2: /* first byte after command */
> if (status == (I2C_SL_IRQ | RNW | RCVD)) {
> - udelay(33);
> + usleep_range(0, 33);
> if (nvec->rx->data[0] != 0x01) {
> dev_err(nvec->dev,
> "Read without prior read command\n");
> @@ -713,7 +713,7 @@ static irqreturn_t nvec_interrupt(int irq, void *dev)
>* We experience less incomplete messages with this delay than without
>* it, but we don't know why. Help is appreciated.
>*/
> - udelay(100);
> + usleep_range(0, 100);
>
> return IRQ_HANDLED;
> }
> --
> 2.7.4
>
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH] CIFS: use the correct length when pinning memory for direct I/O for write
On Sun, Dec 16, 2018 at 11:17:04PM +, Long Li wrote:
> From: Long Li
>
> The current code attempts to pin memory using the largest possible wsize
> based on the currect SMB credits. This doesn't cause kernel oops but this is
> not optimal as we may pin more pages then actually needed.
>
> Fix this by only pinning what are needed for doing this write I/O.
>
> Signed-off-by: Long Li
> Cc: sta...@vger.kernel.org
Looks sane to me.
Reviewed-by: Joey Pabalinas
> ---
> fs/cifs/file.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 3467351..c23bf9d 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2617,11 +2617,13 @@ cifs_write_from_iter(loff_t offset, size_t len,
> struct iov_iter *from,
> if (rc)
> break;
>
> + cur_len = min_t(const size_t, len, wsize);
> +
> if (ctx->direct_io) {
> ssize_t result;
>
> result = iov_iter_get_pages_alloc(
> - from, &pagevec, wsize, &start);
> + from, &pagevec, cur_len, &start);
> if (result < 0) {
> cifs_dbg(VFS,
> "direct_writev couldn't get user pages "
> --
> 2.7.4
>
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH 3/4] rcutorture/nolibc: add a bit of documentation to explain how to use nolibc
On Mon, Dec 31, 2018 at 12:08:54PM -0800, Paul E. McKenney wrote:
> On Sat, Dec 29, 2018 at 09:40:20PM -1000, Joey Pabalinas wrote:
> > On Sun, Dec 30, 2018 at 08:08:46AM +0100, Willy Tarreau wrote:
> > > Definitely! Same, I won't emit a patch just for this, Paul already queued
> > > it.
> >
> > Yeah, not that big a deal :)
> >
> > Reviewed-by: Joey Pabalinas
>
> But as long as I am rebasing to add the Reviewed-by, might as well
> update the others.
>
> The English rules for capitalization and lists are baroque and completely
> unsuited to word processors ("If any list item is longer than one line,
> capitalize all the items."), but in this case each item has multiple
> sentences, so it makes sense to capitalize.
>
> Please see below for the updated commit, and thank you all!
Interesting, I had no idea it was actually that odd, hah.
> And Happy New Year!!! ;-)
Glad to help, and happy new year to you as well :)
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH] KVM: X86: Fix scan ioapic use-before-initialization
On Fri, Dec 28, 2018 at 10:43:11AM +0100, 'Dmitry Vyukov' via syzkaller wrote: > On Thu, Dec 27, 2018 at 6:00 PM Linus Torvalds > wrote: > > Nobody reads the kernel mailing list directly - there's just too much > > traffic. > > As the result bug reports and patches got lots and this is bad and it > would be useful to stop it from happening and there are known ways for > this. What are the "known ways"? The only effective way I can think of is to setup personal email filters for specific topics, and while this is useful and something I do myself, it requires a lot of up front work. I don't think it's realistic to expect others to be doing this instead of just subscribing to the topic lists. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH 2/4] rcutorture/nolibc: fix some poor indentation and alignment
On Sat, Dec 29, 2018 at 07:02:17PM +0100, Willy Tarreau wrote:
> A few macros had their rightmost backslash misaligned, and the pollfd
> struct definition resisted the previous code reindent. Nothing else
> changed.
>
> Cc: Paul E. McKenney
> Signed-off-by: Willy Tarreau
Reviewed-by: Joey Pabalinas
> ---
> tools/testing/selftests/rcutorture/bin/nolibc.h | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/rcutorture/bin/nolibc.h
> b/tools/testing/selftests/rcutorture/bin/nolibc.h
> index 30bd27b..985364c 100644
> --- a/tools/testing/selftests/rcutorture/bin/nolibc.h
> +++ b/tools/testing/selftests/rcutorture/bin/nolibc.h
> @@ -81,9 +81,9 @@ typedef signed longtime_t;
>
> /* for poll() */
> struct pollfd {
> -int fd;
> -short int events;
> -short int revents;
> + int fd;
> + short int events;
> + short int revents;
> };
>
> /* for select() */
> @@ -239,7 +239,7 @@ struct stat {
> "syscall\n" \
> : "=a" (_ret) \
> : "0"(_num) \
> - : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
> \
> + : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
> );\
> _ret; \
> })
> @@ -255,7 +255,7 @@ struct stat {
> : "=a" (_ret) \
> : "r"(_arg1), \
> "0"(_num) \
> - : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
> \
> + : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
> );\
> _ret; \
> })
> @@ -272,7 +272,7 @@ struct stat {
> : "=a" (_ret) \
> : "r"(_arg1), "r"(_arg2), \
> "0"(_num) \
> - : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
> \
> + : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
> );\
> _ret; \
> })
> @@ -290,7 +290,7 @@ struct stat {
> : "=a" (_ret) \
> : "r"(_arg1), "r"(_arg2), "r"(_arg3), \
> "0"(_num) \
> - : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
> \
> + : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
> );\
> _ret; \
> })
> --
> 2.9.0
>
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH 3/4] rcutorture/nolibc: add a bit of documentation to explain how to use nolibc
On Sat, Dec 29, 2018 at 07:02:18PM +0100, Willy Tarreau wrote: > + * - the lower level is the arch-specific syscall() definition, consisting > in > + * assembly code in compound expressions. These are called my_syscall0() > to > + * my_syscall6() depending on the number of arguments. The MIPS > + * implementation is limited to 5 arguments. All input arguments are cast > + * to a long stored in a register. These expressions always return the > + * syscall's return value as a signed long value which is often either a > + * pointer or the negated errno value. > + * > + * - the second level is mostly architecture-independent. It is made of > + * static functions called sys_() which rely on my_syscallN() > + * depending on the syscall definition. These functions are responsible > + * for exposing the appropriate types for the syscall arguments (int, > + * pointers, etc) and for setting the appropriate return type (often > int). > + * A few of them are architecture-specific because the syscalls are not > all > + * mapped exactly the same among architectures. For example, some archs > do > + * not implement select() and need pselect6() instead, so the > sys_select() > + * function will have to abstract this. > + * > + * - the third level is the libc call definition. It exposes the lower raw > + * sys_() calls in a way that looks like what a libc usually does, > + * takes care of specific input values, and of setting errno upon error. > + * There can be minor variations compared to standard libc calls. For > + * example the open() call always takes 3 args here. Shouldn't these sentences begin with a capitalized "The" for consistency? > /* some archs (at least aarch64) don't expose the regular syscalls anymore by > * default, either because they have an "_at" replacement, or because there > are > * more modern alternatives. For now we'd rather still use them. Also here. Shouldn't this begin with a capitalized "Some"? -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH 3/4] rcutorture/nolibc: add a bit of documentation to explain how to use nolibc
On Sun, Dec 30, 2018 at 08:08:46AM +0100, Willy Tarreau wrote: > Definitely! Same, I won't emit a patch just for this, Paul already queued it. Yeah, not that big a deal :) Reviewed-by: Joey Pabalinas -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH AUTOSEL 4.14 27/30] powerpc/selftests: Wait all threads to join
On Sun, Nov 04, 2018 at 08:53:22AM -0500, Sasha Levin wrote: > From: Breno Leitao > > [ Upstream commit 693b31b2fc1636f0aa7af53136d3b49f6ad9ff39 ] > > Test tm-tmspr might exit before all threads stop executing, because it just > waits for the very last thread to join before proceeding/exiting. > > This patch makes sure that all threads that were created will join before > proceeding/exiting. > > This patch also guarantees that the amount of threads being created is equal > to thread_num. > > Signed-off-by: Breno Leitao > Signed-off-by: Michael Ellerman > Signed-off-by: Sasha Levin Acked-by: Joey Pabalinas -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH v4] scripts/tags.sh: use `find` for $ALLSOURCE_ARCHS generation
On Tue, May 22, 2018 at 03:01:07PM +0900, Masahiro Yamada wrote:
> The commit log is wrong.
>
>
> 2018-05-18 20:56 GMT+09:00 Joey Pabalinas :
> > Parsing `ls` is fragile at best and _will_ fail when $tree
> > contains spaces.
>
> This statement is wrong.
>
> The cause of the problem is not using whatever command you use,
> but missing quoting.
> The following would work even if $tree contains spaces:
>
> for arch in `ls "${tree}arch"`; do
Ah, to be completely honest that case didn't even occur to me.
> BTW, what was your motivation of this patch?
>
> Does ${tree} contain spaces?
>
> If the file path contains spaces, the top Makefile terminates it earlier.
>
> Makefile:128: *** main directory cannot contain spaces nor colons. Stop.
It doesn't; I didn't realize the Makefile already had a guard against
spaces in paths. It was something I noticed when poking at something
else and I thought it might be something worth fixing.
But now I agree with you that this patch isn't really needed at all. I can
no longer think of a case where even the original
> for arch in `ls ${tree}arch`; do
would break since spaces are not allowed at all.
Well, on the bright side, the times you happen to be wrong are also the
times where you learn the most :)
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
Re: [PATCH v4] scripts/tags.sh: use `find` for $ALLSOURCE_ARCHS generation
On Tue, May 22, 2018 at 05:41:48PM +0900, Masahiro Yamada wrote:
> I see some new motivations for this patch now.
>
> - The current code includes 'Kconfig' in ALLSOURCE_ARCHS,
> but it should not.
>
> - Simplify the code, removing the find_all_archs().
>
>
> If you are motivated for v5,
> how about this?
>
> - Remove the double-quotes from "${tree}arch/"
> (Because the rest parts of this script do not have double-quoting,
> I do not see point in adding it in this line only.)
>
> - Update the git-log to not mention the hypothetical space things.
Alright, that sounds reasonable. What do you think of:
> ALLSOURCE_ARCHS=$(find ${tree}arch/ -mindepth 1 -maxdepth 1 -type d -printf
> '%f ')
The double quotes surrounding the arguments have been removed to be more
consistent with the other find commands in the script.
The ' %f' was changed to '%f ' so that you don't get that weird leading
space; I couldn't think of a way this could break anything but I wanted
to be 100% sure instead of needing to make a v6, heh.
The commit message will definitely be fixed as well.
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
[PATCH v5] scripts/tags.sh: use `find` for $ALLSOURCE_ARCHS generation
The current code includes 'Kconfig' in ALLSOURCE_ARCHS, but
it should not (Kconfig is not an architecture). Replace this
with a find-generated string and directly assign it to
$ALLSOURCE_ARCHS. The find_all_archs() function is no longer
needed for a one-liner with obvious semantics, so inline the
arch generation into the surrounding conditional.
Signed-off-by: Joey Pabalinas
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/scripts/tags.sh b/scripts/tags.sh
index 78e546ff689c2d5f40..e587610d149271171d 100755
--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -26,24 +26,15 @@ else
fi
# ignore userspace tools
ignore="$ignore ( -path ${tree}tools ) -prune -o"
-# Find all available archs
-find_all_archs()
-{
- ALLSOURCE_ARCHS=""
- for arch in `ls ${tree}arch`; do
- ALLSOURCE_ARCHS="${ALLSOURCE_ARCHS} "${arch##\/}
- done
-}
-
# Detect if ALLSOURCE_ARCHS is set. If not, we assume SRCARCH
if [ "${ALLSOURCE_ARCHS}" = "" ]; then
ALLSOURCE_ARCHS=${SRCARCH}
elif [ "${ALLSOURCE_ARCHS}" = "all" ]; then
- find_all_archs
+ ALLSOURCE_ARCHS=$(find ${tree}arch/ -mindepth 1 -maxdepth 1 -type d
-printf '%f ')
fi
# find sources in arch/$ARCH
find_arch_sources()
{
--
2.17.0.rc1.35.g90bbd502d54fe92035.dirty
signature.asc
Description: PGP signature
Re: [PATCH v5] scripts/tags.sh: use `find` for $ALLSOURCE_ARCHS generation
On Tue, May 22, 2018 at 09:59:02PM +0900, Masahiro Yamada wrote: > > Applied to linux-kbuild. Thanks! Cheers :) -- Joey Pabalinas signature.asc Description: PGP signature
[PATCH] net/tcp/illinois: replace broken algorithm reference link
The link to the pdf containing the algorithm description is now a dead link; it seems http://www.ifp.illinois.edu/~srikant/ has been moved to https://sites.google.com/a/illinois.edu/srikant/ and none of the original papers can be found there... I have replaced it with the only working copy I was able to find. n.b. there is also a copy available at: http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.296.6350&rep=rep1&type=pdf However, this seems to only be a *cached* version, so I am unsure exactly how reliable that link can be expected to remain over time and have decided against using that one. Signed-off-by: Joey Pabalinas 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ipv4/tcp_illinois.c b/net/ipv4/tcp_illinois.c index 7c843578f2333db581..faddf4f9a707f1583f 100644 --- a/net/ipv4/tcp_illinois.c +++ b/net/ipv4/tcp_illinois.c @@ -6,7 +6,7 @@ * The algorithm is described in: * "TCP-Illinois: A Loss and Delay-Based Congestion Control Algorithm * for High-Speed Networks" - * http://www.ifp.illinois.edu/~srikant/Papers/liubassri06perf.pdf + * http://tamerbasar.csl.illinois.edu/LiuBasarSrikantPerfEvalArtJun2008.pdf * * Implemented from description in paper and ns-2 simulation. * Copyright (C) 2007 Stephen Hemminger -- 2.16.2 signature.asc Description: PGP signature
[PATCH] x86/kvm/mmu: const-ify struct kvm_memory_slot pointers
Remove `(struct kvm_memory_slot *)` cast of the
`const struct kvm_memory_slot *memslot` parameter and
const-ify all references to that pointer down the
function call chain.
Signed-off-by: Joey Pabalinas
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index f551962ac29488431b..e6b32de4d7426fecb3 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1235,7 +1235,7 @@ static void pte_list_remove(u64 *spte, struct
kvm_rmap_head *rmap_head)
}
static struct kvm_rmap_head *__gfn_to_rmap(gfn_t gfn, int level,
- struct kvm_memory_slot *slot)
+ const struct kvm_memory_slot *slot)
{
unsigned long idx;
@@ -1678,7 +1678,7 @@ static int kvm_set_pte_rmapp(struct kvm *kvm, struct
kvm_rmap_head *rmap_head,
struct slot_rmap_walk_iterator {
/* input fields. */
- struct kvm_memory_slot *slot;
+ const struct kvm_memory_slot *slot;
gfn_t start_gfn;
gfn_t end_gfn;
int start_level;
@@ -1705,7 +1705,7 @@ rmap_walk_init_level(struct slot_rmap_walk_iterator
*iterator, int level)
static void
slot_rmap_walk_init(struct slot_rmap_walk_iterator *iterator,
- struct kvm_memory_slot *slot, int start_level,
+ const struct kvm_memory_slot *slot, int start_level,
int end_level, gfn_t start_gfn, gfn_t end_gfn)
{
iterator->slot = slot;
@@ -5081,7 +5081,7 @@ typedef bool (*slot_level_handler) (struct kvm *kvm,
struct kvm_rmap_head *rmap_
/* The caller should hold mmu-lock before calling this function. */
static __always_inline bool
-slot_handle_level_range(struct kvm *kvm, struct kvm_memory_slot *memslot,
+slot_handle_level_range(struct kvm *kvm, const struct kvm_memory_slot *memslot,
slot_level_handler fn, int start_level, int end_level,
gfn_t start_gfn, gfn_t end_gfn, bool lock_flush_tlb)
{
@@ -5111,7 +5111,7 @@ slot_handle_level_range(struct kvm *kvm, struct
kvm_memory_slot *memslot,
}
static __always_inline bool
-slot_handle_level(struct kvm *kvm, struct kvm_memory_slot *memslot,
+slot_handle_level(struct kvm *kvm, const struct kvm_memory_slot *memslot,
slot_level_handler fn, int start_level, int end_level,
bool lock_flush_tlb)
{
@@ -5138,7 +5138,7 @@ slot_handle_large_level(struct kvm *kvm, struct
kvm_memory_slot *memslot,
}
static __always_inline bool
-slot_handle_leaf(struct kvm *kvm, struct kvm_memory_slot *memslot,
+slot_handle_leaf(struct kvm *kvm, const struct kvm_memory_slot *memslot,
slot_level_handler fn, bool lock_flush_tlb)
{
return slot_handle_level(kvm, memslot, fn, PT_PAGE_TABLE_LEVEL,
@@ -5245,10 +5245,8 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm,
const struct kvm_memory_slot *memslot)
{
- /* FIXME: const-ify all uses of struct kvm_memory_slot. */
spin_lock(&kvm->mmu_lock);
- slot_handle_leaf(kvm, (struct kvm_memory_slot *)memslot,
-kvm_mmu_zap_collapsible_spte, true);
+ slot_handle_leaf(kvm, memslot, kvm_mmu_zap_collapsible_spte, true);
spin_unlock(&kvm->mmu_lock);
}
--
2.16.2
signature.asc
Description: PGP signature
Re: [PATCH] net/tcp/illinois: replace broken algorithm reference link
On Wed, Feb 28, 2018 at 12:03:58PM -0500, David Miller wrote: > Applied, thank you. No problem, cheers. -- Joey Pabalinas signature.asc Description: PGP signature
[PATCH] sound/pci/ice1712: replace strcpy() with scnprintf()
Replace unsafe uses of strcpy() to copy the name
argument into the sid.name buffer with scnprintf()
to guard against possible buffer overflows.
Even though we don't actually care about the return
value in this specific case, scnprintf() is still
preferred over snprintf() due to scnprintf() returning
the much more logical length of what was *actually* encoded
into the destination array instead of returning length
the resulting string *would* be, assuming it all fit
into the destination array as snprintf() does.
Maybe one day someone will use the return value, and
since the behavior is exactly the same apart from the
return value it would be better to account for that
possibility and program defensively.
Signed-off-by: Joey Pabalinas
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/pci/ice1712/juli.c b/sound/pci/ice1712/juli.c
index 0dbaccf61f33270608..0abacc64168f895d53 100644
--- a/sound/pci/ice1712/juli.c
+++ b/sound/pci/ice1712/juli.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -425,10 +426,9 @@ DECLARE_TLV_DB_SCALE(juli_master_db_scale, -6350, 50, 1);
static struct snd_kcontrol *ctl_find(struct snd_card *card,
const char *name)
{
- struct snd_ctl_elem_id sid;
- memset(&sid, 0, sizeof(sid));
- /* FIXME: strcpy is bad. */
- strcpy(sid.name, name);
+ struct snd_ctl_elem_id sid = {0};
+
+ scnprintf(sid.name, sizeof(sid.name), "%s", name);
sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
return snd_ctl_find_id(card, &sid);
}
diff --git a/sound/pci/ice1712/quartet.c b/sound/pci/ice1712/quartet.c
index d145b5eb7ff86d978d..332f6226548c6a089a 100644
--- a/sound/pci/ice1712/quartet.c
+++ b/sound/pci/ice1712/quartet.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -785,10 +786,9 @@ DECLARE_TLV_DB_SCALE(qtet_master_db_scale, -6350, 50, 1);
static struct snd_kcontrol *ctl_find(struct snd_card *card,
const char *name)
{
- struct snd_ctl_elem_id sid;
- memset(&sid, 0, sizeof(sid));
- /* FIXME: strcpy is bad. */
- strcpy(sid.name, name);
+ struct snd_ctl_elem_id sid = {0};
+
+ scnprintf(sid.name, sizeof(sid.name), "%s", name);
sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
return snd_ctl_find_id(card, &sid);
}
--
2.16.2
signature.asc
Description: PGP signature
Re: [PATCH] sound/pci/ice1712: replace strcpy() with scnprintf()
On Thu, Mar 01, 2018 at 03:55:09PM +0200, Andy Shevchenko wrote: > So, why not just use strlcpy()? > scnprintf() here adds an overhead for no benefit. That's a good point, actually. Revising it now; patch will follow shortly. -- Joey Pabalinas signature.asc Description: PGP signature
[PATCH v2] sound/pci/ice1712: replace strcpy() with strlcpy()
Replace unsafe usages of strcpy() to copy the name
argument into the sid.name buffer with strlcpy()
to guard against possible buffer overflows.
Signed-off-by: Joey Pabalinas
Suggested-by: Andy Shevchenko
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/pci/ice1712/juli.c b/sound/pci/ice1712/juli.c
index 0dbaccf61f33270608..21806bab4757241a9e 100644
--- a/sound/pci/ice1712/juli.c
+++ b/sound/pci/ice1712/juli.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
@@ -425,10 +426,9 @@ DECLARE_TLV_DB_SCALE(juli_master_db_scale, -6350, 50, 1);
static struct snd_kcontrol *ctl_find(struct snd_card *card,
const char *name)
{
- struct snd_ctl_elem_id sid;
- memset(&sid, 0, sizeof(sid));
- /* FIXME: strcpy is bad. */
- strcpy(sid.name, name);
+ struct snd_ctl_elem_id sid = {0};
+
+ strlcpy(sid.name, name, sizeof(sid.name));
sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
return snd_ctl_find_id(card, &sid);
}
diff --git a/sound/pci/ice1712/quartet.c b/sound/pci/ice1712/quartet.c
index d145b5eb7ff86d978d..5bc836241c977feb51 100644
--- a/sound/pci/ice1712/quartet.c
+++ b/sound/pci/ice1712/quartet.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -785,10 +786,9 @@ DECLARE_TLV_DB_SCALE(qtet_master_db_scale, -6350, 50, 1);
static struct snd_kcontrol *ctl_find(struct snd_card *card,
const char *name)
{
- struct snd_ctl_elem_id sid;
- memset(&sid, 0, sizeof(sid));
- /* FIXME: strcpy is bad. */
- strcpy(sid.name, name);
+ struct snd_ctl_elem_id sid = {0};
+
+ strlcpy(sid.name, name, sizeof(sid.name));
sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
return snd_ctl_find_id(card, &sid);
}
--
2.16.2
signature.asc
Description: PGP signature
Re: [PATCH v2] sound/pci/ice1712: replace strcpy() with strlcpy()
On Thu, Mar 01, 2018 at 04:13:48PM +0100, Takashi Iwai wrote: > Thanks, applied now. Cheers -- Joey Pabalinas signature.asc Description: PGP signature
[PATCH] scripts/tags.sh: don't rely on parsing `ls` for $ALLSOURCE_ARCHS generation
Parsing `ls` is fragile at best and _will_ fail when $tree
contains spaces. Replace this with a glob-generated string
and directly assign it to $ALLSOURCE_ARCHS; use a subshell
so `cd` doesn't affect the current working directory.
Signed-off-by: Joey Pabalinas
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/scripts/tags.sh b/scripts/tags.sh
index 78e546ff689c2d5f40..b84acf8889fe836c60 100755
--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -29,14 +29,11 @@ fi
ignore="$ignore ( -path ${tree}tools ) -prune -o"
# Find all available archs
find_all_archs()
{
- ALLSOURCE_ARCHS=""
- for arch in `ls ${tree}arch`; do
- ALLSOURCE_ARCHS="${ALLSOURCE_ARCHS} "${arch##\/}
- done
+ ALLSOURCE_ARCHS="$( (cd "${tree}arch/" && echo *) )"
}
# Detect if ALLSOURCE_ARCHS is set. If not, we assume SRCARCH
if [ "${ALLSOURCE_ARCHS}" = "" ]; then
ALLSOURCE_ARCHS=${SRCARCH}
--
2.17.0.rc1.35.g90bbd502d54fe92035.dirty
signature.asc
Description: PGP signature
[PATCH v3 0/2] tty/nozomi: general module cleanup
The nozomi module has a few sections which could use a bit of cleanup; both style and clarity could be improved while maintaining equivalent semantics. Cleanup messy portions of the module code while preserving existing behavior by: - Replacing constructs like `len__ > TMP_BUF_MAX ? TMP_BUF_MAX : len__` with `min_t(u32, len__, TMP_BUF_MAX)` and function calls like snprintf(tbuf, ..., "%s", ...). with strscpy(tbuf, ..., ...). - Correct inconsistently indented lines and extraneous whitespace. CC: Greg Kroah-Hartman CC: Arnd Bergmann CC: Jiri Slaby Joey Pabalinas (2): tty/nozomi: cleanup DUMP() macro tty/nozomi: fix inconsistent indentation drivers/tty/nozomi.c | 100 +-- 1 file changed, 50 insertions(+), 50 deletions(-) -- 2.17.0.rc1.35.g90bbd502d54fe92035.dirty
[PATCH v3 2/2] tty/nozomi: fix inconsistent indentation
Correct misaligned indentation and remove extraneous spaces.
Signed-off-by: Joey Pabalinas
1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index f26bf1d1e9ee0e74eb..0fcb4db721d2a42f08 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -102,41 +102,41 @@ do {
\
#define RECEIVE_BUF_MAX4
-#define R_IIR 0x /* Interrupt Identity Register */
-#define R_FCR 0x /* Flow Control Register */
-#define R_IER 0x0004 /* Interrupt Enable Register */
+#define R_IIR 0x /* Interrupt Identity Register */
+#define R_FCR 0x /* Flow Control Register */
+#define R_IER 0x0004 /* Interrupt Enable Register */
#define NOZOMI_CONFIG_MAGIC0xEFEFFEFE
#define TOGGLE_VALID 0x
/* Definition of interrupt tokens */
-#define MDM_DL10x0001
-#define MDM_UL10x0002
-#define MDM_DL20x0004
-#define MDM_UL20x0008
-#define DIAG_DL1 0x0010
-#define DIAG_DL2 0x0020
-#define DIAG_UL0x0040
-#define APP1_DL0x0080
-#define APP1_UL0x0100
-#define APP2_DL0x0200
-#define APP2_UL0x0400
-#define CTRL_DL0x0800
-#define CTRL_UL0x1000
-#define RESET 0x8000
+#define MDM_DL10x0001
+#define MDM_UL10x0002
+#define MDM_DL20x0004
+#define MDM_UL20x0008
+#define DIAG_DL1 0x0010
+#define DIAG_DL2 0x0020
+#define DIAG_UL0x0040
+#define APP1_DL0x0080
+#define APP1_UL0x0100
+#define APP2_DL0x0200
+#define APP2_UL0x0400
+#define CTRL_DL0x0800
+#define CTRL_UL0x1000
+#define RESET 0x8000
-#define MDM_DL (MDM_DL1 | MDM_DL2)
-#define MDM_UL (MDM_UL1 | MDM_UL2)
-#define DIAG_DL(DIAG_DL1 | DIAG_DL2)
+#define MDM_DL (MDM_DL1 | MDM_DL2)
+#define MDM_UL (MDM_UL1 | MDM_UL2)
+#define DIAG_DL(DIAG_DL1 | DIAG_DL2)
/* modem signal definition */
-#define CTRL_DSR 0x0001
-#define CTRL_DCD 0x0002
-#define CTRL_RI0x0004
-#define CTRL_CTS 0x0008
+#define CTRL_DSR 0x0001
+#define CTRL_DCD 0x0002
+#define CTRL_RI0x0004
+#define CTRL_CTS 0x0008
-#define CTRL_DTR 0x0001
-#define CTRL_RTS 0x0002
+#define CTRL_DTR 0x0001
+#define CTRL_RTS 0x0002
#define MAX_PORT 4
#define NOZOMI_MAX_PORTS 5
@@ -365,7 +365,7 @@ struct buffer {
u8 *data;
} __attribute__ ((packed));
-/*Global variables */
+/* Global variables */
static const struct pci_device_id nozomi_pci_tbl[] = {
{PCI_DEVICE(0x1931, 0x000c)}, /* Nozomi HSDPA */
{},
@@ -1686,12 +1686,12 @@ static int ntty_tiocmget(struct tty_struct *tty)
/* Note: these could change under us but it is not clear this
matters if so */
- return (ctrl_ul->RTS ? TIOCM_RTS : 0) |
- (ctrl_ul->DTR ? TIOCM_DTR : 0) |
- (ctrl_dl->DCD ? TIOCM_CAR : 0) |
- (ctrl_dl->RI ? TIOCM_RNG : 0) |
- (ctrl_dl->DSR ? TIOCM_DSR : 0) |
- (ctrl_dl->CTS ? TIOCM_CTS : 0);
+ return (ctrl_ul->RTS ? TIOCM_RTS : 0)
+ | (ctrl_ul->DTR ? TIOCM_DTR : 0)
+ | (ctrl_dl->DCD ? TIOCM_CAR : 0)
+ | (ctrl_dl->RI ? TIOCM_RNG : 0)
+ | (ctrl_dl->DSR ? TIOCM_DSR : 0)
+ | (ctrl_dl->CTS ? TIOCM_CTS : 0);
}
/* Sets io controls parameters */
@@ -1722,10 +1722,10 @@ static int ntty_cflags_changed(struct port *port,
unsigned long flags,
const struct async_icount cnow = port->tty_icount;
int ret;
- ret = ((flags & TIOCM_RNG) && (cnow.rng != cprev->rng)) ||
- ((flags & TIOCM_DSR) && (cnow.dsr != cprev->dsr)) ||
- ((flags & TIOCM_CD) && (cnow.dcd != cprev->dcd)) ||
- ((flags & TIOCM_CTS) && (cnow.cts != cprev->cts));
+ ret = ((flags & TIOCM_RNG) && (cnow.rng != cprev->rng))
+ || ((flags & TIOCM_DSR) && (cnow.dsr != cprev->dsr))
+ || ((flags & TIOCM_CD) && (cnow.dcd != cprev->dcd))
+ || ((flags & TIOCM_CTS) && (cnow.cts != cprev->cts));
*cprev = cnow;
--
2.17.0.rc1.35.g90bbd502d54fe92035.dirty
[PATCH v3 1/2] tty/nozomi: cleanup DUMP() macro
Replace snprint() with strscpy() and use min_t() instead of
the conditional operator to clamp buffer length.
Signed-off-by: Joey Pabalinas
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index b57b35066ebea94639..f26bf1d1e9ee0e74eb 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -72,19 +72,19 @@ do {
\
#define TMP_BUF_MAX 256
-#define DUMP(buf__,len__) \
- do { \
-char tbuf[TMP_BUF_MAX] = {0};\
-if (len__ > 1) {\
- snprintf(tbuf, len__ > TMP_BUF_MAX ? TMP_BUF_MAX : len__, "%s", buf__);\
- if (tbuf[len__-2] == '\r') {\
- tbuf[len__-2] = 'r';\
- } \
- DBG1("SENDING: '%s' (%d+n)", tbuf, len__);\
-} else {\
- DBG1("SENDING: '%s' (%d)", tbuf, len__);\
-} \
-} while (0)
+#define DUMP(buf__, len__) \
+ do {\
+ char tbuf[TMP_BUF_MAX] = {0}; \
+ if (len__ > 1) {\
+ u32 data_len = min_t(u32, len__, TMP_BUF_MAX); \
+ strscpy(tbuf, buf__, data_len); \
+ if (tbuf[data_len - 2] == '\r') \
+ tbuf[data_len - 2] = 'r'; \
+ DBG1("SENDING: '%s' (%d+n)", tbuf, len__); \
+ } else {\
+ DBG1("SENDING: '%s' (%d)", tbuf, len__);\
+ } \
+ } while (0)
/*Defines */
#define NOZOMI_NAME"nozomi"
--
2.17.0.rc1.35.g90bbd502d54fe92035.dirty
Re: [PATCH v3 1/2] tty/nozomi: cleanup DUMP() macro
On Wed, Apr 25, 2018 at 07:38:48AM +0200, Greg Kroah-Hartman wrote: > How are you sending these patches? How are you creating them? What is > taking part of the diffstat off and just leaving that line? Hm, my `git format-patch` alias included --shortstat for some odd reason, very sorry about that. Going to resend. -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
[PATCH v4 0/2] tty/nozomi: general module cleanup
The nozomi module has a few sections which could use a bit of cleanup; both style and clarity could be improved while maintaining equivalent semantics. Cleanup messy portions of the module code while preserving existing behavior by: - Replacing constructs like `len__ > TMP_BUF_MAX ? TMP_BUF_MAX : len__` with `min_t(u32, len__, TMP_BUF_MAX)` and function calls like snprintf(tbuf, ..., "%s", ...). with strscpy(tbuf, ..., ...). - Correct inconsistently indented lines and extraneous whitespace. CC: Greg Kroah-Hartman CC: Arnd Bergmann CC: Jiri Slaby Joey Pabalinas (2): tty/nozomi: cleanup DUMP() macro tty/nozomi: fix inconsistent indentation drivers/tty/nozomi.c | 100 +-- 1 file changed, 50 insertions(+), 50 deletions(-) -- 2.17.0.rc1.35.g90bbd502d54fe92035.dirty
