Hi Mark, On May 26 00:09, Mark Geisert wrote: > Hi Corinna, > > Corinna Vinschen wrote: > > Hi Mark, > > > > > On May 22 02:32, Mark Geisert wrote: > > On May 25 14:06, Corinna Vinschen wrote: > > > > Modifies winsup/cygwin/Makefile.in to build localtime.o from items in > > > > new winsup/cygwin/tzcode subdirectory. Compiler option "-fpermissive" > > > > is used to accept warnings about missing casts on the return values of > > > > malloc() calls. This patch also removes existing localtime.cc and > > > > tz_posixrules.h from winsup/cygwin as they are superseded by the > > > > subsequent patches in this set. > > > > [...] > > > > @@ -246,6 +246,15 @@ MATH_OFILES:= \ > > > > tgammal.o \ > > > > truncl.o > > > > +TZCODE_OFILES:=localtime.o > > > > + > > > > +localtime.o: $(srcdir)/tzcode/localtime.cc > > > > $(srcdir)/tzcode/localtime.c.patch > > > > + (cd $(srcdir)/tzcode && \ > > > > + patch -u -o localtime.c.patched localtime.c > > > > localtime.c.patch) > > > > + $(CXX) ${CXXFLAGS} ${localtime_CFLAGS} \ > > > > + -I$(target_builddir)/winsup/cygwin \ > > > > + -I$(srcdir) -I$(srcdir)/tzcode -c -o $@ $< > > > > + > > > > > > This doesn't work well for me. That rule is the top rule in Makefile.in > > > now, so just calling `make' doesn't build the DLL anymore, only > > > localtime.o. The rule should get moved way down Makefile.in. > > Oops. My workflow didn't make this apparent to me. Thanks for the fix. > > > > What still bugs me [...etc...] > > > I attached the followup patches to this mail. Please scrutinize it and > > > don't hesitate to discuss the changes. For a start: > > > > > > - I do not exactly like the name "localtime_wrapper.c" but I don't > > > have a better idea. > > localtime_cygwin.c? cyglocaltime.c? Not much nicer IMO. > > > > - muto's are C++-only, so I changed rwlock_wrlock/rwlock_unlock to use > > > Windows SRWLocks. I think this is a good thing and I'm inclined > > > to drop the muto datatype entirely in favor of using SRWLocks since > > > they are cleaner and langauge-agnostic. > > > > Two changes in my patchset: > > > > - I didn't initialize the SRWLOCK following the books. Fixed that. > > > > - Rather than creating the patched file in the source dir, I changed > > the Makefile.in rule so that the patched file is created in the build > > dir. This drops the requirement to tweak .gitignore. It's also > > cleaner. > > > > - Splitting the build rule for localtime.c.patched from the build rule > > for localtime.o makes sure that the patched file is not regenerated > > every time we build localtime.o. > > > > I attached my patchset again, but only patch 3 and 4 actually changed. > > All the above are great improvements. But I would now remove the "// Get > ready to wrap NetBSD's localtime.c" line and blank line following it.
(Belatedly) done. > Good to go! Great! I added two more tweaks: - I renamed the generated file from localtime.c.patched to localtime.patched.c so the .c suffix remains intact. Seems a bit cleaner to me. I also added it to the 'clean' rule, so that it gets removed at `make clean' time. - I simplified the #includes in the wrapper file. The paths to these headers are searched anyway, so they don't have to be written out explicitely. > Thank you, Good job, thank you! Corinna -- Corinna Vinschen Cygwin Maintainer