Still struggling to get my first patch ready to be committed. I've removed all the typecasts I was able to, fixed variable names, and tried to initialize variables whereever required, as told by people on #winehackers.
Having been told on that channel to only post to wine-patches@ after having the patch reviewed on this list, I'd be grateful for a (hopefully final) review.
Bernhard
2014-07-21 18:58 GMT+02:00 Bernhard Reiter ockham@raz.or.at:
Still struggling to get my first patch ready to be committed. I've removed all the typecasts I was able to, fixed variable names, and tried to initialize variables whereever required, as told by people on #winehackers.
Having been told on that channel to only post to wine-patches@ after having the patch reviewed on this list, I'd be grateful for a (hopefully final) review.
Bernhard
Hi Bernhard,
reiterating what I mentioned on IRC previously i.e. me being clueless about imagehlp, here are some more generic points.
+static int status_routine_called[14] = {0};
No need to explicitly initialize variables with static storage. Not a big deal though.
+ char kernel32_path[MAX_PATH] = "";
No point in initializing it here, the variable is initialized later on anyway. You could potentially check for the return value of GetSystemDirectoryA instead but that's not necessary (most of the callers of GetSystemDirectoryA don't check for that).
- char temp_file[MAX_PATH]; + char temp_file[MAX_PATH] = "";
Same here.
+static void test_bind_image_ex(void) +{ + BOOL ret; + HANDLE file; + char temp_file[MAX_PATH]; + DWORD count; + + /* Call with a non-existant file */ + SetLastError(0xdeadbeef); + ret = pBindImageEx(BIND_NO_BOUND_IMPORTS | BIND_NO_UPDATE | BIND_ALL_IMAGES, temp_file, NULL, + NULL, testing_status_routine);
This one "temp_file" NEEDS to be initialized instead, since you're passing it as a source argument to BindImageEx() and it can potentially contain anything, being a stack-allocated variable. You should probably use an obvious initializer like 'char temp_file[MAX_PATH] = "notexisting.dll";' or something.