Re: Header and comments describing routines in incorrect shape in visibilitymap.c

Hello,

At Wed, 6 Jul 2016 11:28:19 +0900, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote in <CAB7nPqQsQVZbuyjtkGdb5csry-bcp740G2oMPNcQz3yzx4t0xw(at)mail(dot)gmail(dot)com>
> I just bumped into a couple of things in visibilitymap.c:
> - visibilitymap_clear clears all bits of a visibility map, its header
> comment mentions the contrary
> - The header of visibilitymap.c mentions correctly "a bit" when
> referring to setting them, but when clearing, it should say that all
> bits are cleared.
> - visibilitymap_set can set multiple bits
> - visibilitymap_pin can be called to set up more than 1 bit.
>
> This can be summed by the patch attached.

Thank you, I must have been careless on reviewing.

Although some of these are not directly related to 'a bit'
correction, I have some comments on the edited words.

====
- * visibilitymap_pin - pin a map page for setting a bit
+ * visibilitymap_pin - pin a map page for setting bit(s)

Is the parentheses needed? And then pinning is needed not only
for setting bits, but also for clearing. How about the following?

- * visibilitymap_pin - pin a map page for setting a bit
+ * visibilitymap_pin - pin a map page for changing bits

====
- * visibilitymap_set - set a bit in a previously pinned page
+ * visibilitymap_set - set bit(s) in a previously pinned page

So, the 'pinned' is not necessary here or should be added also to
_clear. I think the former is preferable since it is already
written in the comments for the functions and seems to be a bit
too detailed to be put here.

- * visibilitymap_set - set a bit in a previously pinned page
+ * visibilitymap_set - set bits in the visibility map
====

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center