On Fri, 06 Feb 2009, Przemyslaw Czerpak wrote:
Hi,
> > We should IMO never include windows.h from hbdefs.h. Besides
> > slowing down build time to a great deal, we don't do this for other
> > OS APIs either. This would also suggest that Windows API usage
> > is freely allowed in any files, which counteracts with our effort to
> > create portable code in the first place. Most of our core files don't
> > need windows.h.
> I do not agree.
> And now I have very good example.
> In xHarbour there is serious bug related to this subject.
> Today I lost few hours to locate it. The problem can be observed
> in SORT ON ... command but not only. Now I think that I know the
> answer for some other bug reports though without any code watching
> for them.
> Let's give some days to xharbour team to check how much time is
> necessary to locate the source of such problem and fixing it. It's
> possible to fix the problem even without knowing the reason and it
> will be also interesting to see how it will be resolved.
> It should give the real life answer if you are right or not and how
> much developers time can it cost.
And we have an answer:
2009-03-26 10:280 UTC-0430 Ron Pinkas <ron/at/xharbour.com>
* source\rdd\hbdbsort.c
* Synched include files as per dbf1.c as brute force correction to GPF
trap due to structure alignment difference of DBQUICKSORT
/*
NOTE: RDD Developers should seriously cleanup and systemize the RDD
header files,
to avoid such dangerous trap!!!
*/
So exactly the bug in example code was fixed but not the reason
of the problem. It still exists and it's not related to RDD code.
It effects all FS API code which uses _POSIX_PATH_MAX or some structures
like HB_FNAME or HB_FFIND based on above macro.
The bug was introduced over 4 months ago:
2008-12-03 12:10 UTC+0100 Miguel Angel Marchuet <[email protected]>
* include\hbsetup.ch
* include\hbsetup.h
* format & used MAX_PATH or _MAX_PATH instead of hard coded 255, some
systems can support
260 chars and more.
and probably it will not be cleaned for next 4 months or even years.
The example was fixed and probably untill someone else will not create
other one not related to RDD code no one will return to it to try locate
the real reason.
I do not think we should wait longer. It's important for xHarbour users
and developers information so I'm public it.
We also have important information what may happen with partial including
system header files. This declaration in hbsetup.h:
#ifndef _POSIX_PATH_MAX
#if defined( MAX_PATH )
#define _POSIX_PATH_MAX MAX_PATH // 260
#else
#if defined( _MAX_PATH )
#define _POSIX_PATH_MAX _MAX_PATH // 260
#else
#define _POSIX_PATH_MAX 255
#endif
#endif
#endif
is giving different results if windows.h is included before hbsetup.h
or not. In fact we should not make any public definitions which may
depends on some OS settings if we do not include native OS header files
before. In this case it's windows.h but also POSIX systems are not safe
and this bug can be exploited also in Harbour.
_POSIX_PATH_MAX is usually defined indirectly by including limits.h
but not always. It depends on feature macros setting. If some systems
use __BSD_SOURCE as default and does not set any level to __POSIX_SOURCE
then we can redefined _POSIX_PATH_MAX with our own wrong value.
Please think about it.
It's not easy to resolve problem.
In some cases like above it's much safer to use hard coded limits, i.e.:
#define HB_PATH_MAX 264 /* with trailing 0 byte */
In this particular case we should think about updating our code and
replace all
char szFileName[ _POSIX_PATH_MAX + 1 ];
with:
char szFileName[ HB_PATH_MAX ];
264 I chosen intentionally to be large enough for windows (MAX_PATH
it's bigger probably due to drive letter) and to use 8 bytes alignment.
best regards,
Przemek
_______________________________________________
Harbour mailing list
[email protected]
http://lists.harbour-project.org/mailman/listinfo/harbour
