lkml.org 
[lkml]   [2012]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
Date
From
SubjectRe: [PATCH] memcg: Use scnprintf instead of sprintf
On Wed, 18 Apr 2012 11:45:56 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:

> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> This make sure we don't overflow.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> mm/memcontrol.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 519d370..0ccf934 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5269,14 +5269,14 @@ static void mem_cgroup_destroy(struct cgroup *cont)
> }
>
> #ifdef CONFIG_MEM_RES_CTLR_HUGETLB
> -static char *mem_fmt(char *buf, unsigned long n)
> +static char *mem_fmt(char *buf, int size, unsigned long hsize)
> {
> - if (n >= (1UL << 30))
> - sprintf(buf, "%luGB", n >> 30);
> - else if (n >= (1UL << 20))
> - sprintf(buf, "%luMB", n >> 20);
> + if (hsize >= (1UL << 30))
> + scnprintf(buf, size, "%luGB", hsize >> 30);
> + else if (hsize >= (1UL << 20))
> + scnprintf(buf, size, "%luMB", hsize >> 20);
> else
> - sprintf(buf, "%luKB", n >> 10);
> + scnprintf(buf, size, "%luKB", hsize >> 10);
> return buf;
> }

We could use snprintf() here too but it doesn't seem to matter either
way. I guess we _should_ use snprintf as it causes less surprise.

--- a/mm/memcontrol.c~hugetlbfs-add-memcg-control-files-for-hugetlbfs-use-scnprintf-instead-of-sprintf-fix
+++ a/mm/memcontrol.c
@@ -4037,7 +4037,7 @@ static ssize_t mem_cgroup_read(struct cg
BUG();
}

- len = scnprintf(str, sizeof(str), "%llu\n", (unsigned long long)val);
+ len = snprintf(str, sizeof(str), "%llu\n", (unsigned long long)val);
return simple_read_from_buffer(buf, nbytes, ppos, str, len);
}
/*
@@ -5199,11 +5199,11 @@ static void mem_cgroup_destroy(struct cg
static char *mem_fmt(char *buf, int size, unsigned long hsize)
{
if (hsize >= (1UL << 30))
- scnprintf(buf, size, "%luGB", hsize >> 30);
+ snprintf(buf, size, "%luGB", hsize >> 30);
else if (hsize >= (1UL << 20))
- scnprintf(buf, size, "%luMB", hsize >> 20);
+ snprintf(buf, size, "%luMB", hsize >> 20);
else
- scnprintf(buf, size, "%luKB", hsize >> 10);
+ snprintf(buf, size, "%luKB", hsize >> 10);
return buf;
}

It is regrettable that your mem_fmt() exists, especially within
memcontrol.c - it is quite a generic thing. Can't we use
lib/string_helpers.c:string_get_size()? Or if not, modify
string_get_size() so it is usable here?

Speaking of which,

From: Andrew Morton <akpm@linux-foundation.org>
Subject: lib/string_helpers.c: make arrays static

Moving these arrays into static storage shrinks the kernel a bit:

text data bss dec hex filename
723 112 64 899 383 lib/string_helpers.o
516 272 64 852 354 lib/string_helpers.o

Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
lib/string_helpers.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff -puN lib/string_helpers.c~lib-string_helpersc-make-arrays-static lib/string_helpers.c
--- a/lib/string_helpers.c~lib-string_helpersc-make-arrays-static
+++ a/lib/string_helpers.c
@@ -23,15 +23,15 @@
int string_get_size(u64 size, const enum string_size_units units,
char *buf, int len)
{
- const char *units_10[] = { "B", "kB", "MB", "GB", "TB", "PB",
+ static const char *units_10[] = { "B", "kB", "MB", "GB", "TB", "PB",
"EB", "ZB", "YB", NULL};
- const char *units_2[] = {"B", "KiB", "MiB", "GiB", "TiB", "PiB",
+ static const char *units_2[] = {"B", "KiB", "MiB", "GiB", "TiB", "PiB",
"EiB", "ZiB", "YiB", NULL };
- const char **units_str[] = {
+ static const char **units_str[] = {
[STRING_UNITS_10] = units_10,
[STRING_UNITS_2] = units_2,
};
- const unsigned int divisor[] = {
+ static const unsigned int divisor[] = {
[STRING_UNITS_10] = 1000,
[STRING_UNITS_2] = 1024,
};
_


\
 
 \ /
  Last update: 2012-04-19 00:39    [W:0.074 / U:56.464 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site