[lvm-devel] [PATCH 07/23] Move lastfd handling into for()

[Milan Broz]

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