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.
_______________________________________________
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make
