For review please: wineps.drv: Implement PS_USERSTYLE on printers

[Bruno Jesus]

Hi, Daniel.

On Thu, Oct 13, 2011 at 06:14, Daniel <danielfsantos at 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...