From: Milan Broz <mbroz@redhat.com>
To: lvm-devel@redhat.com
Subject: [PATCH 07/23] Move lastfd handling into for()
Date: Tue, 21 Dec 2010 17:50:09 +0100 [thread overview]
Message-ID: <4D10DAC1.9050708@redhat.com> (raw)
In-Reply-To: <403be35b0f1d1d1da149e3d9a948a6416475287e.1292945707.git.zkabelac@redhat.com>
On 12/21/2010 04:41 PM, Zdenek Kabelac wrote:
> @@ -826,7 +826,8 @@ static void main_loop(int local_sock, int cmd_timeout)
>
> if (thisfd->removeme) {
> struct local_client *free_fd;
> - lastfd->next = thisfd->next;
> + if (lastfd)
> + lastfd->next = thisfd->next;
Isn't that replacing one bug with another?
If lastfd is NULL, it will crash.
After your change it will quietly do something, apparently with wrong pointers.
If lastfd cannot be NULL, the check is not needed and is redundant.
I *think* that the first fd in select is local one, removing itself is nonsense,
so lastfd should be always set. (lastfd is set in the for cycle)
Anyway I prefer to keep the code intact instead of hiding possible bug.
(Or rewrite it to something more readable. e.g. removeme flag
seems to be used only in gulm code, which is dead and unsupported etc)
Milan
next prev parent reply other threads:[~2010-12-21 16:50 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-21 15:41 [PATCH 00/23] Static analyzis set Zdenek Kabelac
2010-12-21 15:41 ` [PATCH 01/23] Remove dead assignment to thisfd Zdenek Kabelac
2011-01-05 2:40 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 02/23] Remove dead assignment Zdenek Kabelac
2011-01-05 2:44 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 03/23] Remove dead assigment of mirr_state Zdenek Kabelac
2011-01-05 2:38 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 04/23] Remove dead assignment of lock_flags Zdenek Kabelac
2010-12-21 16:20 ` Milan Broz
2010-12-22 10:11 ` Zdenek Kabelac
2011-01-05 0:30 ` Alasdair G Kergon
2011-01-05 0:57 ` Milan Broz
2010-12-21 15:41 ` [PATCH 05/23] Remove dead assignment in vgconvert_single Zdenek Kabelac
2011-01-05 1:11 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 06/23] Log of pthread_join operation Zdenek Kabelac
2011-01-05 2:36 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 07/23] Move lastfd handling into for() Zdenek Kabelac
2010-12-21 16:50 ` Milan Broz [this message]
2010-12-22 10:16 ` Zdenek Kabelac
2011-01-05 0:28 ` Alasdair G Kergon
2011-01-05 1:06 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 08/23] Fix memory leak in error path of restart_clvmd Zdenek Kabelac
2010-12-21 15:41 ` [PATCH 09/23] Add test for errors in _setup_task Zdenek Kabelac
2010-12-21 17:12 ` Milan Broz
2010-12-22 10:23 ` Zdenek Kabelac
2011-01-05 0:24 ` Alasdair G Kergon
2011-01-05 2:03 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 10/23] Fix memory leak of dev_filter on error path Zdenek Kabelac
2010-12-21 17:21 ` Milan Broz
2010-12-22 10:05 ` Zdenek Kabelac
2011-01-05 0:35 ` Alasdair G Kergon
2011-01-05 1:02 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 11/23] Add stack traces for archive Zdenek Kabelac
2010-12-21 15:41 ` [PATCH 12/23] Add tests for dm_task_set operations Zdenek Kabelac
2010-12-21 15:41 ` [PATCH 13/23] Make it obvious this code is not used Zdenek Kabelac
2010-12-21 15:41 ` [PATCH 14/23] Add stack trace for backup and backup_remove Zdenek Kabelac
2010-12-21 15:41 ` [PATCH 15/23] Add test check for find_config_str Zdenek Kabelac
2010-12-21 17:32 ` Milan Broz
2010-12-22 10:00 ` Zdenek Kabelac
2011-01-05 0:49 ` Alasdair G Kergon
2011-01-05 1:01 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 16/23] Remove check for vg != NULL Zdenek Kabelac
2010-12-21 15:41 ` [PATCH 17/23] Add check for dm_snprintf result Zdenek Kabelac
2010-12-21 17:38 ` Milan Broz
2010-12-22 10:34 ` Zdenek Kabelac
2011-01-05 0:12 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 18/23] Return PERCENT_INVALID for error case Zdenek Kabelac
2010-12-21 17:47 ` Milan Broz
2011-01-05 2:33 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 19/23] Warning - dead code problem elimination Zdenek Kabelac
2010-12-21 17:55 ` Milan Broz
2010-12-22 9:35 ` Petr Rockai
2011-01-05 2:15 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 20/23] exit if lvmchache_init fails Zdenek Kabelac
2011-01-05 1:22 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 21/23] Add default error path for get_property Zdenek Kabelac
2011-01-05 2:05 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 22/23] Remove dead assignment in dm_event_get_registered_device Zdenek Kabelac
2011-01-05 2:13 ` Alasdair G Kergon
2010-12-21 15:41 ` [PATCH 23/23] Remove dead assignment in vg_split_mdas Zdenek Kabelac
2011-01-05 1:59 ` Alasdair G Kergon
2010-12-21 18:09 ` [PATCH 00/23] Static analyzis set Milan Broz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D10DAC1.9050708@redhat.com \
--to=mbroz@redhat.com \
--cc=lvm-devel@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.