On 09/01/13 00:34, Pete Batard wrote:
> On 2013.01.08 14:56, Toby Gray wrote:
>> In anticipation of this, I've just rebased and tested my wince[1] branch
>> on the latest libusbx master.
> Thanks. I'm picking it up and starting to look at it a bit more closely.
> What's most likely to happen is that I'm going to merge most of your
> commits into a handful of large ones.
>
> I still have yet to test compilation (both for and outside of CE), but
> below are a few points I'd like to bring up:
>
> 1. in .gitignore, there's a !wince/listdevs entry. Shouldn't there be
> similar entries for xusb and test? Also what does the 'STANDARDSDK_500
> (*)' entry do?

Sorry, those are .gitignore entries left over from before I changed 
where the binaries ended up. They can safely be removed.

> 2. I see you using both a WINCE and _WIN32_WCE define. Is there a
> difference between the two? If not, maybe it'd make sense to just pick
> one and try to stick to it. Also, if a file tends to use #if defined()
> rather than #ifdef, it's probably worth carrying the trend (looking at
> the #ifndef WINCE at the top of libusb.h)

Agreed. The choice between the two defines is entirely arbitrary.

> 3. As you may have noticed, with the VS2012 addon, I did some renaming
> of the solution files (such as libusb_static.vcproj ->
> libusb_static_2005.vcproj). Ultimately, if we add a suffix to your
> project files and use a few well placed #ifdefs, I believe we could
> probably move the wince solution and extra include into the /msvc
> directory. I think the config.h should be able to service both msvc
> wince and regular msvc with a handful of extra #ifdef WINCE, and the
> project files should be a no brainer either. The only unknown potential
> issue I can see is if we get to pick errno.h from msvc/ for MinGW/cygwin
> compilation instead of the system's one, but there's probably a way to
> avoid that. I don't see merging msvc_wince and msvc as a priority at
> this stage, but we might want to keep that option open.

I'm happy to attempt to merge the two if having all the VS build files 
in the same directory makes sense. I think it'd make sense to do the 
directory merge sooner rather than latter, as otherwise it leads to 
unnecessary confusion to people moving from one release to another.

> 4. Ideally, instead of using #ifdef WIN_CE in the core source, we'd
> probably want to emulate the HAS_SYS_TYPE_H, HAS_SIGNAL_H functionally
> from autoconf in the msvc's config.h. We're already populating some of
> these values on anything POSIX, and can add the missing ones if needed,
> so I might just do that during integration. Of course, we can't use the
> HAS_#### values in all files (libusb.h and the samples/tests come to
> mind, as they don't rely on config.h), but it would probably help making
> our code look a bit more conventional.

I'd entirely forgotten about using HAS_SYS_TYPE_H, using it would 
certainly be neater.

> (5. Note to self: add windows_common.h to the VS2012 solution files)
>
> 6. Did we have a potential bug in poll_windows.c in
> _fd_to_index_and_lock ("if (fd <= 0)" -> "if (fd < 0)"? If so thanks for
> the fix. Overall, I think your changes made poll_windows.c a lot easier
> to read, which should help.

It would only be a bug if fd 0 has been closed and is then reused. Even 
then I'm not sure if Windows will ever reuse the fd values for STDIN, 
STDOUT and STDERR.

> 7. Maybe you want to edit INSTALL_WIN.txt to add some notes about WinCE.

Yes, documentation would be a good idea.

Do you want me to make the changes you've suggested in my branch and 
then let you know? Or should I wait and then provide patches on top of 
what ends up in libusbx.org git repository?

Regards,

Toby

------------------------------------------------------------------------------
Master Java SE, Java EE, Eclipse, Spring, Hibernate, JavaScript, jQuery
and much more. Keep your Java skills current with LearnJavaNow -
200+ hours of step-by-step video tutorials by Java experts.
SALE $49.99 this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122612 
_______________________________________________
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Reply via email to