Johannes Sixt wrote:
> On Samstag, 5. Dezember 2009, Ramsay Jones wrote:
>> The patch is still marked RFC because:
>> - I'm still not sure if the flexibility to support both 32- and 64-bit
>> time_t is required.
>> - should -D_USE_32BIT_TIME_T be added to the Makefile?
>
> If *not* using -D_USE_32BIT_TIME_T produces a build or code base that is in
> some way superior, why should we require it? For example, its absence could
> help a 64bit build.
Indeed.
I only added the second bullet because you seemed to be advocating it, in your
last email, so I'm waiting for your answer on that. :-D
Unfortunately the patch can not be a simple as the first version (Dscho was
quite right to complain about adding too much clutter to mingw.h with msvc
related code), but the moral equivalent would have msvc.h look like:
25 /* Use mingw_lstat() instead of lstat()/stat() and mingw_fstat() instead
26 * of fstat(). We add the declaration of these functions here, suppressing
27 * the corresponding declarations in mingw.h, so that we can use the
28 * appropriate structure type (and function) names from the msvc headers.
29 */
30 #define stat _stat64
31 int mingw_lstat(const char *file_name, struct stat *buf);
32 int mingw_fstat(int fd, struct stat *buf);
33 #define fstat mingw_fstat
34 #define lstat mingw_lstat
35 #define _stat64(x,y) mingw_lstat(x,y)
36 #define ALREADY_DECLARED_STAT_FUNCS
37
38 #include "compat/mingw.h"
39
40 #undef ALREADY_DECLARED_STAT_FUNCS
This works fine, *provided* you do not need to compile with -D_USE_32BIT_TIME_T,
which would produce this warning:
...mingw.c(223) : warning C4133: 'function' : incompatible types - \
from '_stat64 *' to '_stat32i64 *'
This would actually be *worse* than the original code, since the struct _stat64
would not have the same "shape" as the struct _stat32i64; it would not write
outside of the allocated memory, at least, but the time fields would obviously
be written to the wrong offsets etc,. In the original code, the struct _stati64
would have the correct "shape", since the time fields are declared with time_t.
At first I thought there would be no need to set _USE_32BIT_TIME_T. After some
thought, however, I could not be confident that it would *never* be necessary.
(my only concern was to revert to 32-bit time_t, perhaps only temporarily, while
fixing a breakage; I did not consider wanting to keep "compatibility" with the
MinGW code). This lead to the more complicated/flexible RFC patch.
I was hoping for someone to say: "Hey, we will *never* need to compile with
-D_USE_32BIT_TIME_T, so just get rid of those #if conditionals and simplify
the code ...". :-P
Since nobody has said any such thing, I have to conclude that the extra
flexibility is required. That being the case, I have to be happy with the
patch as-is and propose to remove the RFC.
Before I do that, do you have any further comments or concerns about the
v2 patch that you want me to address?
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html