Hi, Daniel. On Thu, Oct 13, 2011 at 06:14, Daniel <danielfsan...@att.net> wrote: > Hello guys. I would like to get a little review on this patch, a follow > up to the patches that fix this problem on the screen. Specifically, > I'm most concerned about making sure my dynamically allocated memory is > freed under all conditions and there's no potential snafus that I'm not > aware of. Then, of course, if somebody sees something off, please do > comment.
+ int i; + EXTLOGPEN *elp = NULL; + INT size = GetObjectW( hpen, 0, NULL ); + + if (!size) return 0; + else if (size == sizeof(logpen)) { + GetObjectW( hpen, sizeof(logpen), &logpen ); + } else { Alexandre replied in your patch email regarding this part of the patch, take a look at it. + const char *pattern = "%d"; /* don't put a space in front of the 1st number */ Are you sure const is doing what you think it is? + pattern = " %d"; You're setting the pattern var inside the loop, isn't it better to start with "%d " and trim the string after the for loop? + if (elp) HeapFree( GetProcessHeap(), 0, elp ); There is no need to check elp before calling HeapFree, it takes care of null pointers. + BOOL own_dash; /* TRUE if we need to free dash */ You are using spaces, the rest of the variable declarations use tabs. I usually don't feel safe about reviewing patches so I would appreciate if anyone care to review my review. Best wishes, Bruno -- universe* god::bigbang (void); //and then it all began...