|
Boost : |
From: Mike Morearty (mike_at_[hidden])
Date: 2000-01-11 13:17:03
I just discovered Boost, so my apologies if some of these comments have been
made before. A quick search through the old postings didn't turn up similar
comments.
I downloaded the dir_it library from http://www.boost.org. It's a very nice
class, but I have a number of small suggestions & bug fixes for its use on
Windows platforms.
Most importantly, there's a bug in the Windows implementation of
get<user_execute>(). This function currently will crash the program if the
filename has no extension at all (e.g. filename foo, as opposed to foo.c,
foo.exe, etc.). This is because it calls name.find_last_of('.') without
checking whether the return value is equal to std::string::npos, which is the
return code indicating failure.
Also, a less important bug: the old ".com" extension from DOS does still
work, and does still indicate an executable.
Here's a version which fixes these problems:
template <> get<user_execute>::operator user_execute::value_type()
const
{
std::string name(*m_it);
std::string::size_type dot = name.find_last_of('.');
if (dot == std::string::npos)
return false;
else
{
std::string ext(name.substr(dot));
return ext == ".exe" || ext == ".com" || ext == ".bat";
}
}
-- My next suggestion is that the code refer to "Win32" instead of "WinNT" throughout. The code works just fine on Windows 95 and Windows 98 (I tried it). In general, most WinNT code which doesn't use security and doesn't use lower-level OS features will work on Win9x. "Win32" is a generic term which can be applied to all of these platforms (Win95, Win98, WinNT, Win2000). So, for example, everwhere that you use "BOOST_WINNT", I'd suggest it be changed to "BOOST_WIN32". WinNT is also mentioned in comments in boost/boost.h and src/directory.cpp. These could be changed to refer to Win32 instead. -- Related to this: src/directory.cpp has the following conditional near the top of the file: #if defined(unix) || defined(__unix) || defined(__unix__) # define BOOST_UNIX 1 #elif defined(_WINDOWS) // <-- This should be _WIN32 # define BOOST_WINNT 1 #endif _WINDOWS should be changed to _WIN32. This is kind of subtle and confusing. With the Visual C++ integrated development environment, when you create a project for any GUI application, it will define _WINDOWS for that application. However, that does not indicate Windows in general -- rather, it indicates a GUI application as opposed to a console application. If you use the development environment to create a console application, it will not define _WINDOWS -- instead, it will define _CONSOLE. _WIN32, on the other hand, is defined by the compiler itself (not the development environment). That means _WIN32 will always work for everyone who uses Visual C++, even if they wrote their own makefile by hand, or used the development environment to create a console app. -- Last suggestion: In Windows DLLs, you are not required to define the DllMain function. If you omit it, the Microsoft libraries will provide it for you. Since, in the case of dir_it, DllMain is empty, the project will be a little cleaner if you just leave out the function (and in fact the whole boost.cpp file, since that's the only function in the file). That's it! Thanks again for a useful class. - Mike Morearty <mike_at_[hidden]>
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk