On Thu, Oct 23, 2008 at 03:54:59PM +0100, Andy Whitcroft wrote:
> gethugepagesizes has two modes: a counting mode which tells us exactly
> how many entries we would need; and a lookup mode which fills in a
> supplied buffer.
>
> The counting mode is defined as triggering when we pass in both a zero
> element count and a NULL buffer pointer. Currently we enter this mode when
> the element count is zero which causes it to trigger at the edge case where
> the caller (pointlessly) passes in a buffer with an element count of zero.
> Fix behaviour to match the documentation.
>
> Signed-off-by: Andy Whitcroft <[EMAIL PROTECTED]>
> ---
> hugeutils.c | 19 ++++++++++---------
> 1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/hugeutils.c b/hugeutils.c
> index ae60f39..eb0b7b2 100644
> --- a/hugeutils.c
> +++ b/hugeutils.c
> @@ -581,11 +581,11 @@ int gethugepagesizes(long pagesizes[], int n_elem)
> if (default_size < 0)
> return 0;
>
> - if (n_elem && pagesizes)
> + if (pagesizes && (nr_sizes == n_elem))
> + return nr_sizes;
Ok, this is a little confusing because nr_sizes == 0 - can't we just go
pagesizes && n_elem == 0 ?
Otherwise, the behaviour appears to match the documentation. If you
supply a buffer with zero elements, then 0 pagesizes are put in the
buffer.
> + if (pagesizes)
> pagesizes[nr_sizes] = default_size;
I'm not seeing why you have to check pagesizes here. Earlier in the
function, we already check it for EINVAL and the check above would exit
if n_elem was 0 so we know the array is valid at this point, right?
> nr_sizes++;
> - if (n_elem && (nr_sizes == n_elem))
> - return nr_sizes;
>
> /*
> * Scan sysfs to look for other sizes.
> @@ -599,8 +599,7 @@ int gethugepagesizes(long pagesizes[], int n_elem)
> } else
> return -1;
> }
> - while ((ent = readdir(sysfs)) &&
> - ((n_elem == 0) || (nr_sizes < n_elem))) {
> + while ((ent = readdir(sysfs))) {
> long size;
>
> if (strncmp(ent->d_name, "hugepages-", 10))
> @@ -613,7 +612,9 @@ int gethugepagesizes(long pagesizes[], int n_elem)
>
> if (size < 0 || size == default_size)
> continue;
> - if (n_elem && pagesizes)
> + if (pagesizes && (nr_sizes == n_elem))
> + return nr_sizes;
> + if (pagesizes)
> pagesizes[nr_sizes] = size;
Can we really reach here with pagesizes == NULL?
> nr_sizes++;
> }
> @@ -636,10 +637,10 @@ int getpagesizes(long pagesizes[], int n_elem)
> ret = gethugepagesizes(pagesizes, n_elem);
> } else {
> /* Install the base page size. */
> - if (n_elem && pagesizes)
> + if (pagesizes && n_elem == 0)
> + return 0;
Here the test is easier to follow.
> + if (pagesizes)
> pagesizes[0] = sysconf(_SC_PAGESIZE);
> - if (n_elem == 1)
> - return 1;
>
> ret = gethugepagesizes(pagesizes + 1, n_elem - 1);
> }
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Libhugetlbfs-devel mailing list
Libhugetlbfs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libhugetlbfs-devel
