* Asias He <asias.hejun@xxxxxxxxx> wrote:
> - fd = open(filename, O_RDONLY);
> + /*
> + * Be careful! We are opening host block device!
> + * Open it readonly since we do not want to break user's data on disk.
> + */
> + fd = open(filename, O_RDONLY);
> if (fd < 0)
> return NULL;
btw., this is a repeating pattern i noticed: you align assignment vertically
even if it's a stand-alone assignment.
We want to apply vertical alignment only when it helps readability - and
repeated assingments such as:
*job = (struct thread_pool__job) {
.kvm = kvm,
.data = data,
.callback = callback,
.mutex = PTHREAD_MUTEX_INITIALIZER
};
indeed look *much* better when aligned vertically. Same goes for structure
definitions. Thanks for applying those concepts uniformly around tools/kvm/,
it makes the code visibly more pleasant to read.
But the above standalone assignment of 'fd' does not seem to be such a case:
the right side of the assignment just 'floats' freely in space with no other
similar assingment next to it giving it structure.
Thus the old-fashioned:
fd = open(filename, O_RDONLY);
if (fd < 0)
return NULL;
is a lot more readable form IMO.
There's many similar examples of isolated assignments looking weird, all around
the code.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
