On Wed, 21 Nov 2012 04:05:19 +0000 (GMT) 박성진 <sj76.p...@samsung.com> said:
ok - i didn't get started with your first one... so i'll start here. note - i havent dug in depth and looked all over libxinput2.2 itself... 1. Xi2_2 -> is libXi2.2 really libXi2_2.so..... ? really? this has ADDED stuff from xi2... but should it have a whole new soname on real systems? 2. XI_2_Major and XI_2_Minor -> i would guess that the headers of xinput2 define these.. are these new in 2.2 or were they there before? this smells fishy to me. 3. just style-wise... pleasde don't do: if (EINA_TRUE != find || !touchdev->slot) instead: if ((!find) || (!touchdev->slot)) notice i'm using the booleanness of find ANd using ()'s to group logic checks rather than precedence of operation. :) just a small optimization - you use a while loop to walk thru _ecore_x_xi2_touch - you also made it a linked list as well.. why dont you use eina_list, or eina_inlist ? inlist inlines the lit node info into the struct... also something i tend to do with such lists that dont have any specific sort-order... when i do a successful lookup, i put the found item at the list start so the theory goes - commonly accessed items are found very quickly as they are at the font. unless real life usage is that you get pretty much random usage evenly distributed... then there is no benefit... for this i suspect it helps a little. especially once we get like 5 or 10 devices... and we look this up often.. which is what we do for every touch event there. :) more consistent formatting would be nice. ie static Ecore_X_Touch_Device_Info* _ecore_x_input_touch_info_get()... is all one line but others are on 2 lines... make it consistent so its easier to read :) formatting: if(touchdev) -> if (touchdev) ... (notice the space) touchdev->slot can be made part of the calloc of touchdev - alloc 1 memory blob that is sizeof Ecore_X_Touch_Device_Info PLUS ((t->num_touches) * sizeof (int))... and make slot an array of 1 instead of int *. ie int slot[1]; instead of int *slot; otherwise looks ok to me. can you fix the above and re-submit? :) tnx muchly! > Dear developers, > > I found some of my debug routines are existing in my patch and I removed them > and added exception codes. I attached my newer patch. > > Thanks and regards, > Sung-Jin Park > > ------- Original Message ------- > Sender : 박성진<sj76.p...@samsung.com> S5(책임)/책임/System S/W Lab(S/W센터)/ > 삼성전자 Date : 2012-11-19 20:33 (GMT+09:00) > Title : Review request : [Ecore XI2] Add codes for selecting/retrieving XI2 > touch events > > Dear developers, > I added codes for selecting XI2 touch events(cf. XI_TouchBegin, > XI_TouchUpdate and XI_TouchEnd) and codes for retrieving touched finger(s) > index from corresponding events. > > Please kindly review my codes. > All comments will be welcomed. :) > > Thanks and regards, > Sung-Jin Park<p> </p><p> </p> -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) ras...@rasterman.com ------------------------------------------------------------------------------ Monitor your physical, virtual and cloud infrastructure from a single web console. Get in-depth insight into apps, servers, databases, vmware, SAP, cloud infrastructure, etc. Download 30-day Free Trial. Pricing starts from $795 for 25 servers or applications! http://p.sf.net/sfu/zoho_dev2dev_nov _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
