Re: [bug #41758] VMS Make incorrectly reports archives support present.

[h.becker]

On 05/15/2014 05:51 AM, John Malmberg wrote:
> Follow-up Comment #4, bug #41758 (project make):
>

I admit I never looked at all the archive/library code before. So I may
need some education. The patch works for my simple test but that worked
with the unpatched version as well. So what's really different? Or which
test cases are addressed?

However, the patch doesn't work if I dare to specify a module name
different from the file name. That worked with the unpatched version.

On VAX/VMS 7.3, the version I run in simh on my laptop (when I have no
access to the net) I get:
%SYSTEM-F-ACCVIO, access violation, reason mask=00, virtual
address=00000000, PC=00000000, PSL=03C00000

Compiling arscan with /debug/noopt doesn't show any problem, but with
/debug only I get
DBG> set rad hex; set module arscan; set break VMS_get_member_info
DBG> g
break at routine ARSCAN\VMS_get_member_info
 13214: static int
DBG> g
%DEBUG-W-BADSTACK, WARNING: stack corrupted; session integrity not
guaranteed
%SYSTEM-F-ACCVIO, access violation, reason mask=04, virtual
address=0000000C, PC=00138DCF, PSL=03C00004

It seems this happens in/around ar_member_date_1, which is called from
VMS_get_member_info

DBG> set rad hex; set module ar; set module arscan
DBG> set break VMS_get_member_info; set break ar_member_date_1
DBG> g
break at routine ARSCAN\VMS_get_member_info
 13214: static int
 DBG> sh call
  module name    routine name     line                rel PC
abs PC
*ARSCAN         VMS_get_member_info
                                13214               000000B2
00029862
 SHARE$LBRSHR                                       00003764
00055D64
 SHARE$LBRSHR                                       00003DAA
000563AA
 SHARE$LBRSHR                                       00003DFD
000563FD
 SHARE$LBRSHR                                       00003608
00055C08
*ARSCAN         ar_scan         13414               000002E6
00029A96
*AR             ar_member_date   9407               000001E1
00029591
*REMAKE         f_mtime         15421               00002E47
0001BF87
*REMAKE         update_file_1   14666               000014A9
0001A5E9
*REMAKE         update_file     14445               00000D33
00019E73
*REMAKE         update_goal_chain
                                14284               0000097D
00019ABD
*MAIN           main            12504               000036BE
00012066
DBG> g
break at routine AR\ar_member_date_1
DBG> sh call
 module name    routine name     line                rel PC
abs PC
*AR             ar_member_date_1
                                 9369               0000012E
000294DE
DBG> step/return
stepped on return from routine AR\ar_member_date_1 to
AR\ar_member_date_1\%LINE 9377
DBG> step
stepped to ARSCAN\VMS_get_member_info\%LINE 13308+1B
 13308:   VMS_function_ret =
 DBG> sh call
  module name    routine name     line                rel PC
abs PC
DBG>


Anyway, some comments ...

Having descriptive names for some variables and using const is good. But
when introducing different mechanisms/algorithms such changes
make it more difficult to see what's really new. Maybe having two
patches would help, one for the code improvements and one for the "real"
changes.

There are some new conditionals for VMS in the source code. I understand
that they are required to have VMS specific code. But it would make the
source more readable if such code could be moved into VMS specific
sources or sections within the source file. I didn't try to do that nor
do I have a real plan, it is just an idea to make this code easier to
understand.

It seems that the existing and new code for VMS is complex, at least
callbacks are not straight forward and deserve some good comments.
Rather than commenting on the lines, like "On VMS, there is probably a
saved suffix..." I suggest to have one big comment to explain the whole
picture, examples included. Then, someone looking at the VMS specific
code doesn't need to look at all the comments in all the conditional
code to understand the overall picture. Also, someone who isn't
interested in VMS code will have shorter source code sections to skip
(OK, I should have known that everybody is using Eclipse and has folding
for preprocessor branches enabled :-)

I suggest to remove the (old) comments on decc$fix_time, it looks like
decc$fix_time is always needed (and not only on VAX and Alpha). And the
old comments are in git anyway.

For the tests, the perl scripts, I suggest to change $vms, which is
explained as '# $vms is for VMS with out a GNV Bash shell'. Then this is
very likely DCL mode, or? So it should be named either DCL[_mode], or
for GNV and bash there should be a flag named GNV[_mode|_bash]. I prefer
the latter just because DCL is default for VMS and GNV bash is special.

The test for imagelib seems to work on VAX only: I only see a UNIVERSAL
option and not a SYMBOL_VECTOR option.

For utouch in the perl scripts there is a comment saying "utouch is not
changing what VMS library compare is testing for". I'm not sure I
understand what the "VMS library compare" is in this context. I would
like to have the comment saying what utouch is changing and what is
compared. I assume it is the VMS modfication aka revision time, which is
compared. For what it is worth, I do have a utouch.c [.exe] which
changes the VMS modfication time by an offset - the same interface as
the perl subroutine.