On Wed, 2011-02-23 at 07:32 +0900, sola wrote:
From 7784102e184c134eaa11b5986f31c892748f1604 Mon Sep 17 00:00:00 2001 From: Masashi YOKOTA yokota@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.1980.a@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