| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 30 Sep 2014 12:34:35 +0200
From: Petr Mladek <pmladek@...e.cz>
To: Joe Perches <joe@...ches.com>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Al Viro <viro@...IV.linux.org.uk>,
Andrew Morton <akpm@...ux-foundation.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Christine Caulfield <ccaulfie@...hat.com>,
David Teigland <teigland@...hat.com>, cluster-devel@...hat.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/7] dlm: Use seq_is_full - remove seq_printf returns
On Mon 29-09-14 16:08:23, Joe Perches wrote:
> The seq_printf return should be ignored and the
> seq_is_full function should be tested instead.
>
> Convert functions returning int to void where
> seq_printf is used.
>
> Signed-off-by: Joe Perches <joe@...ches.com>
> ---
> fs/dlm/debug_fs.c | 248 +++++++++++++++++++++++++-----------------------------
> 1 file changed, 116 insertions(+), 132 deletions(-)
>
> diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
> index 1323c56..27c8335 100644
> --- a/fs/dlm/debug_fs.c
> +++ b/fs/dlm/debug_fs.c
> @@ -48,8 +48,8 @@ static char *print_lockmode(int mode)
> }
> }
>
> -static int print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
> - struct dlm_rsb *res)
> +static void print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
> + struct dlm_rsb *res)
> {
> seq_printf(s, "%08x %s", lkb->lkb_id, print_lockmode(lkb->lkb_grmode));
>
> @@ -68,21 +68,17 @@ static int print_format1_lock(struct seq_file *s, struct dlm_lkb *lkb,
> if (lkb->lkb_wait_type)
> seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
>
> - return seq_puts(s, "\n");
> + seq_puts(s, "\n");
> }
>
> -static int print_format1(struct dlm_rsb *res, struct seq_file *s)
> +static void print_format1(struct dlm_rsb *res, struct seq_file *s)
> {
> struct dlm_lkb *lkb;
> int i, lvblen = res->res_ls->ls_lvblen, recover_list, root_list;
> - int rv;
>
> lock_rsb(res);
>
> - rv = seq_printf(s, "\nResource %p Name (len=%d) \"",
> - res, res->res_length);
> - if (rv)
> - goto out;
> + seq_printf(s, "\nResource %p Name (len=%d) \"", res, res->res_length);
To keep the functionality, we should add
if(seq_overflow(s))
goto out;
> for (i = 0; i < res->res_length; i++) {
> if (isprint(res->res_name[i]))
> @@ -92,17 +88,16 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
> }
>
> if (res->res_nodeid > 0)
> - rv = seq_printf(s, "\"\nLocal Copy, Master is node %d\n",
> - res->res_nodeid);
> + seq_printf(s, "\"\nLocal Copy, Master is node %d\n",
> + res->res_nodeid);
> else if (res->res_nodeid == 0)
> - rv = seq_puts(s, "\"\nMaster Copy\n");
> + seq_puts(s, "\"\nMaster Copy\n");
> else if (res->res_nodeid == -1)
> - rv = seq_printf(s, "\"\nLooking up master (lkid %x)\n",
> - res->res_first_lkid);
> + seq_printf(s, "\"\nLooking up master (lkid %x)\n",
> + res->res_first_lkid);
> else
> - rv = seq_printf(s, "\"\nInvalid master %d\n",
> - res->res_nodeid);
> - if (rv)
> + seq_printf(s, "\"\nInvalid master %d\n", res->res_nodeid);
> + if (seq_is_full(s))
> goto out;
I would check for seq_overflow()
Etc. There are needed many more changes if we agree on introducing
seq_is_full() and seq_overflow().
Well, we will need to touch most of these locations once again when
Steven removes return value from all the seq_* functions.
Best Regards,
Petr
> /* Print the LVB: */
> @@ -116,8 +111,8 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
> }
> if (rsb_flag(res, RSB_VALNOTVALID))
> seq_puts(s, " (INVALID)");
> - rv = seq_puts(s, "\n");
> - if (rv)
> + seq_puts(s, "\n");
> + if (seq_is_full(s))
> goto out;
> }
>
> @@ -125,32 +120,30 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
> recover_list = !list_empty(&res->res_recover_list);
>
> if (root_list || recover_list) {
> - rv = seq_printf(s, "Recovery: root %d recover %d flags %lx "
> - "count %d\n", root_list, recover_list,
> - res->res_flags, res->res_recover_locks_count);
> - if (rv)
> - goto out;
> + seq_printf(s, "Recovery: root %d recover %d flags %lx count %d\n",
> + root_list, recover_list,
> + res->res_flags, res->res_recover_locks_count);
> }
>
> /* Print the locks attached to this resource */
> seq_puts(s, "Granted Queue\n");
> list_for_each_entry(lkb, &res->res_grantqueue, lkb_statequeue) {
> - rv = print_format1_lock(s, lkb, res);
> - if (rv)
> + print_format1_lock(s, lkb, res);
> + if (seq_is_full(s))
> goto out;
> }
>
> seq_puts(s, "Conversion Queue\n");
> list_for_each_entry(lkb, &res->res_convertqueue, lkb_statequeue) {
> - rv = print_format1_lock(s, lkb, res);
> - if (rv)
> + print_format1_lock(s, lkb, res);
> + if (seq_is_full(s))
> goto out;
> }
>
> seq_puts(s, "Waiting Queue\n");
> list_for_each_entry(lkb, &res->res_waitqueue, lkb_statequeue) {
> - rv = print_format1_lock(s, lkb, res);
> - if (rv)
> + print_format1_lock(s, lkb, res);
> + if (seq_is_full(s))
> goto out;
> }
>
> @@ -159,23 +152,23 @@ static int print_format1(struct dlm_rsb *res, struct seq_file *s)
>
> seq_puts(s, "Lookup Queue\n");
> list_for_each_entry(lkb, &res->res_lookup, lkb_rsb_lookup) {
> - rv = seq_printf(s, "%08x %s", lkb->lkb_id,
> - print_lockmode(lkb->lkb_rqmode));
> + seq_printf(s, "%08x %s",
> + lkb->lkb_id, print_lockmode(lkb->lkb_rqmode));
> if (lkb->lkb_wait_type)
> seq_printf(s, " wait_type: %d", lkb->lkb_wait_type);
> - rv = seq_puts(s, "\n");
> + seq_puts(s, "\n");
> + if (seq_is_full(s))
> + goto out;
> }
> out:
> unlock_rsb(res);
> - return rv;
> }
>
> -static int print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
> - struct dlm_rsb *r)
> +static void print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
> + struct dlm_rsb *r)
> {
> u64 xid = 0;
> u64 us;
> - int rv;
>
> if (lkb->lkb_flags & DLM_IFL_USER) {
> if (lkb->lkb_ua)
> @@ -188,103 +181,97 @@ static int print_format2_lock(struct seq_file *s, struct dlm_lkb *lkb,
> /* id nodeid remid pid xid exflags flags sts grmode rqmode time_us
> r_nodeid r_len r_name */
>
> - rv = seq_printf(s, "%x %d %x %u %llu %x %x %d %d %d %llu %u %d \"%s\"\n",
> - lkb->lkb_id,
> - lkb->lkb_nodeid,
> - lkb->lkb_remid,
> - lkb->lkb_ownpid,
> - (unsigned long long)xid,
> - lkb->lkb_exflags,
> - lkb->lkb_flags,
> - lkb->lkb_status,
> - lkb->lkb_grmode,
> - lkb->lkb_rqmode,
> - (unsigned long long)us,
> - r->res_nodeid,
> - r->res_length,
> - r->res_name);
> - return rv;
> + seq_printf(s, "%x %d %x %u %llu %x %x %d %d %d %llu %u %d \"%s\"\n",
> + lkb->lkb_id,
> + lkb->lkb_nodeid,
> + lkb->lkb_remid,
> + lkb->lkb_ownpid,
> + (unsigned long long)xid,
> + lkb->lkb_exflags,
> + lkb->lkb_flags,
> + lkb->lkb_status,
> + lkb->lkb_grmode,
> + lkb->lkb_rqmode,
> + (unsigned long long)us,
> + r->res_nodeid,
> + r->res_length,
> + r->res_name);
> }
>
> -static int print_format2(struct dlm_rsb *r, struct seq_file *s)
> +static void print_format2(struct dlm_rsb *r, struct seq_file *s)
> {
> struct dlm_lkb *lkb;
> - int rv = 0;
>
> lock_rsb(r);
>
> list_for_each_entry(lkb, &r->res_grantqueue, lkb_statequeue) {
> - rv = print_format2_lock(s, lkb, r);
> - if (rv)
> + print_format2_lock(s, lkb, r);
> + if (seq_is_full(s))
> goto out;
> }
>
> list_for_each_entry(lkb, &r->res_convertqueue, lkb_statequeue) {
> - rv = print_format2_lock(s, lkb, r);
> - if (rv)
> + print_format2_lock(s, lkb, r);
> + if (seq_is_full(s))
> goto out;
> }
>
> list_for_each_entry(lkb, &r->res_waitqueue, lkb_statequeue) {
> - rv = print_format2_lock(s, lkb, r);
> - if (rv)
> + print_format2_lock(s, lkb, r);
> + if (seq_is_full(s))
> goto out;
> }
> out:
> unlock_rsb(r);
> - return rv;
> }
>
> -static int print_format3_lock(struct seq_file *s, struct dlm_lkb *lkb,
> +static void print_format3_lock(struct seq_file *s, struct dlm_lkb *lkb,
> int rsb_lookup)
> {
> u64 xid = 0;
> - int rv;
>
> if (lkb->lkb_flags & DLM_IFL_USER) {
> if (lkb->lkb_ua)
> xid = lkb->lkb_ua->xid;
> }
>
> - rv = seq_printf(s, "lkb %x %d %x %u %llu %x %x %d %d %d %d %d %d %u %llu %llu\n",
> - lkb->lkb_id,
> - lkb->lkb_nodeid,
> - lkb->lkb_remid,
> - lkb->lkb_ownpid,
> - (unsigned long long)xid,
> - lkb->lkb_exflags,
> - lkb->lkb_flags,
> - lkb->lkb_status,
> - lkb->lkb_grmode,
> - lkb->lkb_rqmode,
> - lkb->lkb_last_bast.mode,
> - rsb_lookup,
> - lkb->lkb_wait_type,
> - lkb->lkb_lvbseq,
> - (unsigned long long)ktime_to_ns(lkb->lkb_timestamp),
> - (unsigned long long)ktime_to_ns(lkb->lkb_last_bast_time));
> - return rv;
> + seq_printf(s, "lkb %x %d %x %u %llu %x %x %d %d %d %d %d %d %u %llu %llu\n",
> + lkb->lkb_id,
> + lkb->lkb_nodeid,
> + lkb->lkb_remid,
> + lkb->lkb_ownpid,
> + (unsigned long long)xid,
> + lkb->lkb_exflags,
> + lkb->lkb_flags,
> + lkb->lkb_status,
> + lkb->lkb_grmode,
> + lkb->lkb_rqmode,
> + lkb->lkb_last_bast.mode,
> + rsb_lookup,
> + lkb->lkb_wait_type,
> + lkb->lkb_lvbseq,
> + (unsigned long long)ktime_to_ns(lkb->lkb_timestamp),
> + (unsigned long long)ktime_to_ns(lkb->lkb_last_bast_time));
> }
>
> -static int print_format3(struct dlm_rsb *r, struct seq_file *s)
> +static void print_format3(struct dlm_rsb *r, struct seq_file *s)
> {
> struct dlm_lkb *lkb;
> int i, lvblen = r->res_ls->ls_lvblen;
> int print_name = 1;
> - int rv;
>
> lock_rsb(r);
>
> - rv = seq_printf(s, "rsb %p %d %x %lx %d %d %u %d ",
> - r,
> - r->res_nodeid,
> - r->res_first_lkid,
> - r->res_flags,
> - !list_empty(&r->res_root_list),
> - !list_empty(&r->res_recover_list),
> - r->res_recover_locks_count,
> - r->res_length);
> - if (rv)
> + seq_printf(s, "rsb %p %d %x %lx %d %d %u %d ",
> + r,
> + r->res_nodeid,
> + r->res_first_lkid,
> + r->res_flags,
> + !list_empty(&r->res_root_list),
> + !list_empty(&r->res_recover_list),
> + r->res_recover_locks_count,
> + r->res_length);
> + if (seq_is_full(s))
> goto out;
>
> for (i = 0; i < r->res_length; i++) {
> @@ -300,8 +287,8 @@ static int print_format3(struct dlm_rsb *r, struct seq_file *s)
> else
> seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
> }
> - rv = seq_puts(s, "\n");
> - if (rv)
> + seq_puts(s, "\n");
> + if (seq_is_full(s))
> goto out;
>
> if (!r->res_lvbptr)
> @@ -311,58 +298,55 @@ static int print_format3(struct dlm_rsb *r, struct seq_file *s)
>
> for (i = 0; i < lvblen; i++)
> seq_printf(s, " %02x", (unsigned char)r->res_lvbptr[i]);
> - rv = seq_puts(s, "\n");
> - if (rv)
> + seq_puts(s, "\n");
> + if (seq_is_full(s))
> goto out;
>
> do_locks:
> list_for_each_entry(lkb, &r->res_grantqueue, lkb_statequeue) {
> - rv = print_format3_lock(s, lkb, 0);
> - if (rv)
> + print_format3_lock(s, lkb, 0);
> + if (seq_is_full(s))
> goto out;
> }
>
> list_for_each_entry(lkb, &r->res_convertqueue, lkb_statequeue) {
> - rv = print_format3_lock(s, lkb, 0);
> - if (rv)
> + print_format3_lock(s, lkb, 0);
> + if (seq_is_full(s))
> goto out;
> }
>
> list_for_each_entry(lkb, &r->res_waitqueue, lkb_statequeue) {
> - rv = print_format3_lock(s, lkb, 0);
> - if (rv)
> + print_format3_lock(s, lkb, 0);
> + if (seq_is_full(s))
> goto out;
> }
>
> list_for_each_entry(lkb, &r->res_lookup, lkb_rsb_lookup) {
> - rv = print_format3_lock(s, lkb, 1);
> - if (rv)
> + print_format3_lock(s, lkb, 1);
> + if (seq_is_full(s))
> goto out;
> }
> out:
> unlock_rsb(r);
> - return rv;
> }
>
> -static int print_format4(struct dlm_rsb *r, struct seq_file *s)
> +static void print_format4(struct dlm_rsb *r, struct seq_file *s)
> {
> int our_nodeid = dlm_our_nodeid();
> int print_name = 1;
> - int i, rv;
> + int i;
>
> lock_rsb(r);
>
> - rv = seq_printf(s, "rsb %p %d %d %d %d %lu %lx %d ",
> - r,
> - r->res_nodeid,
> - r->res_master_nodeid,
> - r->res_dir_nodeid,
> - our_nodeid,
> - r->res_toss_time,
> - r->res_flags,
> - r->res_length);
> - if (rv)
> - goto out;
> + seq_printf(s, "rsb %p %d %d %d %d %lu %lx %d ",
> + r,
> + r->res_nodeid,
> + r->res_master_nodeid,
> + r->res_dir_nodeid,
> + our_nodeid,
> + r->res_toss_time,
> + r->res_flags,
> + r->res_length);
>
> for (i = 0; i < r->res_length; i++) {
> if (!isascii(r->res_name[i]) || !isprint(r->res_name[i]))
> @@ -377,10 +361,9 @@ static int print_format4(struct dlm_rsb *r, struct seq_file *s)
> else
> seq_printf(s, " %02x", (unsigned char)r->res_name[i]);
> }
> - rv = seq_puts(s, "\n");
> - out:
> + seq_puts(s, "\n");
> +
> unlock_rsb(r);
> - return rv;
> }
>
> struct rsbtbl_iter {
> @@ -390,11 +373,12 @@ struct rsbtbl_iter {
> int header;
> };
>
> -/* seq_printf returns -1 if the buffer is full, and 0 otherwise.
> - If the buffer is full, seq_printf can be called again, but it
> - does nothing and just returns -1. So, the these printing routines
> - periodically check the return value to avoid wasting too much time
> - trying to print to a full buffer. */
> +/*
> + * If the buffer is full, seq_printf can be called again, but it
> + * does nothing. So, the these printing routines periodically check
> + * seq_is_full to avoid wasting too much time trying to print to
> + * a full buffer.
> + */
>
> static int table_seq_show(struct seq_file *seq, void *iter_ptr)
> {
> @@ -403,7 +387,7 @@ static int table_seq_show(struct seq_file *seq, void *iter_ptr)
>
> switch (ri->format) {
> case 1:
> - rv = print_format1(ri->rsb, seq);
> + print_format1(ri->rsb, seq);
> break;
> case 2:
> if (ri->header) {
> @@ -412,21 +396,21 @@ static int table_seq_show(struct seq_file *seq, void *iter_ptr)
> "r_nodeid r_len r_name\n");
> ri->header = 0;
> }
> - rv = print_format2(ri->rsb, seq);
> + print_format2(ri->rsb, seq);
> break;
> case 3:
> if (ri->header) {
> seq_printf(seq, "version rsb 1.1 lvb 1.1 lkb 1.1\n");
> ri->header = 0;
> }
> - rv = print_format3(ri->rsb, seq);
> + print_format3(ri->rsb, seq);
> break;
> case 4:
> if (ri->header) {
> seq_printf(seq, "version 4 rsb 2\n");
> ri->header = 0;
> }
> - rv = print_format4(ri->rsb, seq);
> + print_format4(ri->rsb, seq);
> break;
> }
>
> --
> 1.8.1.2.459.gbcd45b4.dirty
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists