lkml.org 
[lkml]   [2012]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] cpumask: add a few comments of cpumask functions
On 05/28/2012 02:32 PM, Alex Shi wrote:

> Current few cpumask function purpose are not quite clear. Stupid
> user like myself need to dig into details for clear function
> purpose and return value.


You can just see how it is used elsewhere and figure it out ;-)
Anyway, in principle, I don't have any objections to adding comments
that are actually helpful. But I don't think all the comments this
patch adds fall into that category..

> Add few explanation for them is helpful.
>
> Signed-off-by: Alex Shi <alex.shi@intel.com>
> ---
> include/linux/cpumask.h | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index a2c819d..8436e61 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -271,6 +271,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
> * cpumask_test_cpu - test for a cpu in a cpumask
> * @cpu: cpu number (< nr_cpu_ids)
> * @cpumask: the cpumask pointer
> + * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
> *


s/return/Returns

What do you mean by "old" cpumask?

> * No static inline type checking - see Subtlety (1) above.
> */
> @@ -281,6 +282,7 @@ static inline void cpumask_clear_cpu(int cpu, struct cpumask *dstp)
> * cpumask_test_and_set_cpu - atomically test and set a cpu in a cpumask
> * @cpu: cpu number (< nr_cpu_ids)
> * @cpumask: the cpumask pointer
> + * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
> *


"Test and Set" already has a very well-defined and well-known meaning.. Not sure if the
comment adds any extra value..

> * test_and_set_bit wrapper for cpumasks.
> */
> @@ -293,6 +295,7 @@ static inline int cpumask_test_and_set_cpu(int cpu, struct cpumask *cpumask)
> * cpumask_test_and_clear_cpu - atomically test and clear a cpu in a cpumask
> * @cpu: cpu number (< nr_cpu_ids)
> * @cpumask: the cpumask pointer
> + * return 1 if the 'cpu' is in the old 'cpumask', otherwise return 0
> *


Same here. Pretty well known meaning.

> * test_and_clear_bit wrapper for cpumasks.
> */
> @@ -324,6 +327,7 @@ static inline void cpumask_clear(struct cpumask *dstp)
> * @dstp: the cpumask result
> * @src1p: the first input
> * @src2p: the second input
> + * if *dstp is empty, return 0, otherwise return 1


I don't think anybody would be interested in the return value of this function!
The functionality (rather than the return value) is what is more interesting
and useful, and is already well-documented.

> */
> static inline int cpumask_and(struct cpumask *dstp,
> const struct cpumask *src1p,
> @@ -365,6 +369,7 @@ static inline void cpumask_xor(struct cpumask *dstp,
> * @dstp: the cpumask result
> * @src1p: the first input
> * @src2p: the second input
> + * if *dstp is empty, return 0, otherwise return 1


Same here. Functionality is more interesting.

> */
> static inline int cpumask_andnot(struct cpumask *dstp,
> const struct cpumask *src1p,
> @@ -414,6 +419,7 @@ static inline bool cpumask_intersects(const struct cpumask *src1p,
> * cpumask_subset - (*src1p & ~*src2p) == 0
> * @src1p: the first input
> * @src2p: the second input
> + * return 1 if the *src1p is the subset of *src2p, otherwise return 0


s/return/Returns
(I think the function name itself is pretty descriptive, but anyway...)

> */
> static inline int cpumask_subset(const struct cpumask *src1p,
> const struct cpumask *src2p)



Regards,
Srivatsa S. Bhat



\
 
 \ /
  Last update: 2012-05-28 15:21    [W:0.395 / U:0.540 seconds]
©2003-2014 Jasper Spaans. Advertise on this site