Re: vfs: Don't copy mount bind mounts of /proc/<pid>/ns/mnt between namespaces

[ebiederm@xmission.com (Eric W. Biederman)]

Dan Carpenter <dan.carpenter@oracle.com> writes:

> On Mon, Sep 02, 2013 at 01:30:00PM -0700, Eric W. Biederman wrote:
>> Dan Carpenter <dan.carpenter@oracle.com> writes:
>> 
>> > Hello Eric W. Biederman,
>> >
>> > This is a semi-automatic email about new static checker warnings.
>> >
>> > The patch 4ce5d2b1a8fd: "vfs: Don't copy mount bind mounts of
>> > /proc/<pid>/ns/mnt between namespaces" from Mar 30, 2013, leads to
>> > the following Smatch complaint:
>> 
>> Semi-autoautomatic reply.  The test !q is enough to ensure p is valid
>> until p->mnt.mnt_root == q->mnt.mnt_root.
>> 
>> This has been verified through both testing and reading of the code.
>> 
>
> Why not make the test:
>
> 	while (1) { ...

I really don't understand what you are getting at, and it feels
like this conversation is descending into inanity fast.

The way this work is.
p = old
q = new

Where new is a copy of old.

p == NULL means that we have completed the traversal.

The entire point of the loop is to find corresponding entities in new
and old.  Because in a few correr cases I slice off leaf mounts that
are mounted on top of files I need an extra loop, and I made that with
a very small code change.

So yes it does look like the case that except for the first iteration
the test at the top of the while loop is now redundant, because of the
if (!q) break; that I added.

I think any greater change to the loop would have obscured the change
that I made a great deal more.

I think all of this talking about the syntactical elements of the code
instead of brining this up to the level of human beings and talking
about what is going on is extremely dangerous way to look at code.
Especially given that C does not allow everything of importance about
the code to be expressed.

Eric