On Wed, 2011-06-22 at 14:23 -0500, Seth Jennings wrote:
> +#ifdef CONFIG_SYSFS
There are a couple of #ifdef CONFIG_SYSFS blocks in zcache.c already.
Could this go inside one of those instead of being off by itself?
> +static int zv_show_pool_pages_count(char *buf)
> +{
> + char *p = buf;
> + unsigned long numpages;
> +
> + if (zcache_client.xvpool == NULL)
> + p += sprintf(p, "%d\n", 0);
> + else {
^^ That's probably a good spot to include brackets. They don't take up
any more lines, and it keeps folks from introducing bugs doing things
like:
if (zcache_client.xvpool == NULL)
p += sprintf(p, "%d\n", 0);
bar();
else {
> + numpages = xv_get_total_size_bytes(zcache_client.xvpool);
> + p += sprintf(p, "%lu\n", numpages >> PAGE_SHIFT);
> + }
In this case 'numpages' doesn't actually store a number of pages; it
stores a number of bytes. I'd probably rename it.
Also 'numpages' is an 'unsigned long' while xv_get_total_size_bytes()
returns a u64. 'unsigned long' is only 32-bits on 32-bit architectures,
so it's possible that large buffers sizes could overflow. The easiest
way to fix this is probably to just make 'numpages' a u64.
-- Dave
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>