On Wed, 2011-02-23 at 07:32 +0900, sola wrote:
> From 7784102e184c134eaa11b5986f31c892748f1604 Mon Sep 17 00:00:00 2001
> From: Masashi YOKOTA <yok...@pylone.jp>
> Date: Sat, 19 Feb 2011 02:40:56 +0900
> Subject: [PATCH 1/2] Add virtual battery driver
>
> This patch adds virtual battery driver.
> This is based on
> git://git.linaro.org/people/jstultz/linux.git dev/linaro.android
>
> Signed-off-by: Akihiro MAEDA <sola.198...@gmail.com>
Hey Akihiro,
So I applied your patches, but I ran into a few problematic spots.
1) Your patch was whitespace corrupted. While it was correctly sent
inline, the patch was word-wrapped. I was able to fix this, but most
upstream maintainers won't take the time, so you'll want to fix this in
the future when sending patches. It looks from the mailheaders that you
were using gmail to send the patch? You might look at the wiki page
here for help setting up git-send-email:
https://wiki.linaro.org/Mentoring/Git/GitSendEmail
2) After applying the patch and trying a test build, I saw the following
warning:
drivers/power/virtual_battery.c: In function 'map_get_value':
drivers/power/virtual_battery.c:180: warning: the frame size of 4096
bytes is larger than 1024 bytes
>From the code:
> +static int map_get_value(struct battery_property_map * map, const
> char * key, int def_val)
> +{
> + char buf[4096];
> + int cr;
> +
So yea. We definitely don't wan 4k on the stack here. This doesn't seem
reasonable at all. Also probably should be using strncpy below as well.
> + strcpy(buf, key);
> + cr = strlen(buf) - 1;
> + if (buf[cr] == '\n')
> + buf[cr] = '\0';
> +
> + while (map->key) {
> + if (strcasecmp(map->key, buf) == 0)
> + return map->value;
> + map++;
> + }
> +
> + return def_val;
> +}
I'll try to add some cleanups here, but on closer inspection the patch
may need a bit more work to be ready to go upstream.
thanks
-john
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
