On Tue, Aug 4, 2015 at 5:45 PM, Robert Haas <
robertmhaas@gmail.com> wrote:
>
> On Tue, Jul 28, 2015 at 3:28 PM, Heikki Linnakangas <
hlinnaka@iki.fi> wrote:
> > * The patch requires that the LWLOCK_INDIVIDUAL_NAMES array is kept in sync
> > with the list of individual locks in lwlock.h. Sooner or later someone will
> > add an LWLock and forget to update the names-array. That needs to be made
> > less error-prone, so that the names are maintained in the same place as the
> > #defines. Perhaps something like rmgrlist.h.
>
> This is a good idea, but it's not easy to do in the style of
> rmgrlist.h, because I don't believe there's any way to define a macro
> that expands to a preprocessor directive. Attached is a patch that
> instead generates the list of macros from a text file, and also
> generates an array inside lwlock.c with the lock names that gets used
> by the Trace_lwlocks stuff where applicable.
>
> Any objections to this solution to the problem? If not, I'd like to
> go ahead and push this much. I can't test the Windows changes
> locally, though, so it would be helpful if someone could check that
> out.
>
Okay, I have check it on Windows and found below issues which I
have fixed in patch attached with this mail.
Use of uninitialized value in numeric lt (<) at Solution.pm line 103.
Could not open src/backend/storage/lmgr/lwlocknames.h at Mkvcbuild.pm line 674.
+ if (IsNewer(
+ 'src/backend/storage/lmgr/lwlocknames.txt',
'src/include/storage/lwlocknames.h'))
I think the usage of old and new filenames in IsNewer is reversed here.
2.
+ print "Generating lwlocknames.c and fmgroids.h...\n";
Here it should be lwlocknames.h instead of fmgroids.h
3.
+ if (IsNewer(
+ 'src/include/storage/lwlocknames.h',
+
'src/backend/storage/lmgr/fmgroids.h'))
Here, it seems lwlocknames.h needs to compared instead of fmgroids.h
Generating lwlocknames.c and fmgroids.h...
/* autogenerated from src/backend/storage/lmgr/lwlocknames.txt, do not edit */
/* there is deliberately not an #ifndef LWLOCKNAMES_H here */
59, <$lwlocknames> line 47.
rename($htmp, 'lwlocknames.h') || die "rename: $htmp: $!";
The reason is that closing of tmp files is missing.
Some other general observations
5.
On running perl
mkvcbuild.pl in Windows, below message is generated.
Generating lwlocknames.c and lwlocknames.h...
/* autogenerated from src/backend/storage/lmgr/lwlocknames.txt, do not edit */
/* there is deliberately not an #ifndef LWLOCKNAMES_H here */
Following message gets displayed, which looks slightly odd, displaying
it in C comments makes it look out of place and other similar .pl files
don't generate such message. Having said that, I think it displays useful
information, so it might be okay to retain it.
6.
+
+maintainer-clean: clean
+ rm -f lwlocknames.h
Here, don't we need to clean lwlocknames.c?
