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.
It does indeed completely fix http://bugs.winehq.org/show_bug.cgi?id=26091 (printed a cute piece of music with dashed lines).
Thanks all! Daniel
Hi, Daniel.
On Thu, Oct 13, 2011 at 06:14, Daniel danielfsantos@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
-
Bruno Jesus -
Daniel