On Wed, 10 Oct 2012, Christophe escribió: > > ----- Rodolfo García Peñas (kix) <[email protected]> a écrit : > > On 2012-10-09 01:08, Christophe wrote: > > > Hi Rodolfo, > > > > > > While having a quick look at this patch, I noticed something that > > > looks plain wrong to me in the original code: > > > > > > ----- Rodolfo García Peñas <[email protected]> a écrit : > > >> > > >> From a280e1aa0fe1e2500f6f687b7b195cbb2bca9ba2 Mon Sep 17 00:00:00 > > >> 2001 > > >> From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?= > > >> <[email protected]> > > >> Date: Sat, 6 Oct 2012 18:18:43 +0200 > > >> Subject: [PATCH 2/3] wcore set vmask in one step > > >> > > >> [...] > > >> > > >> - vmask = CWBorderPixel | CWCursor | CWEventMask | > > >> CWOverrideRedirect; > > >> + vmask = CWBorderPixel | CWCursor | CWEventMask | > > >> CWOverrideRedirect | CWColormap; > > >> attribs.override_redirect = True; > > >> attribs.cursor = wCursor[WCUR_DEFAULT]; > > >> attribs.background_pixmap = None; > > > > > > the attribs 'background_pixmap' and 'background_pixel' but the > > > corresponding flags (CWBackPixmap and CWBackPixel) are not set (but > > > that's not really a problem). > > > > > > > > >> @@ -99,14 +98,13 @@ WCoreWindow *wCoreCreate(WCoreWindow *parent, > > >> int x, int y, int width, int heigh > > >> > > >> - vmask = CWBorderPixel | CWCursor | CWEventMask; > > >> + vmask = CWBorderPixel | CWCursor | CWEventMask | CWColormap; > > >> attribs.cursor = wCursor[WCUR_DEFAULT]; > > >> attribs.background_pixmap = None; > > >> attribs.background_pixel = parent->screen_ptr->black_pixel; > > >> attribs.event_mask = KeyPressMask | KeyReleaseMask | > > >> ButtonPressMask | > > >> ButtonReleaseMask | ButtonMotionMask | > > >> ExposureMask | EnterWindowMask | > > >> LeaveWindowMask; > > >> - vmask |= CWColormap; > > >> attribs.colormap = parent->screen_ptr->w_colormap; > > > > > > This time it's a bit worse, because it uses flag 'CWBorderPixel' but > > > does not set the field 'border_pixel'. It also sets fields > > > 'background_*' with no use as the corresponding flags are missing. > > > > > > Probably no risk of crash, just a wrong border color on some (old) > > > display. > > > > > > Regards, > > > Christophe. > > > > Hi Christophe, > > > > thanks for your review. I have a doubt about your comments. > > > > In the original code: > > > > 1. Set vmask > > 2. Set attribs (and attribs flags) > > 3. vmask changed (|=) > > > > But vmask and attribs (and the flags) are not used yet (only assigned). > > So, we can do (the patch): > > > > 1. Set vmask (|= included) > > 2. Set attribs. > > > > I cannot see the problem :-( I am missing something? > > Hi! > No, you're not missing anything! > Your patch is 100% correct, the problem is in some code that you have not > changed, I just saw it because it was visible through your patch. > I'm sending a patch to fix it so you'll be able to judge by yourself.
Ah! OK, thanks :-) > Best regards, > Christophe. > > > > > Thanks! > > kix > > > > -- > > ||// //\\// Rodolfo "kix" Garcia > > ||\\// //\\ http://www.kix.es/ > > > > > > -- > > To unsubscribe, send mail to [email protected]. > > > -- > To unsubscribe, send mail to [email protected]. -- ||// //\\// Rodolfo "kix" Garcia ||\\// //\\ http://www.kix.es/ -- To unsubscribe, send mail to [email protected].
