[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: John E. Malmberg
Subject: Re: [bug #41758] VMS Make incorrectly reports archives support present.
Date: Sun, 01 Jun 2014 23:22:43 -0500
User-agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:17.0) Gecko/20130215 Thunderbird/17.0.3

On 5/31/2014 7:21 AM, h.becker wrote:
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?

This fixes 8 failing tests in the features/archives test. Wildcard access now works correctly. In addition the vpath3 test that was failing before now works correctly. It needs to be modified to expect the result in VMS syntax.

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.

I need to look at that to see how it could have worked, since GNU Make is what is assuming that the module name is the same as the filename, and I do not see a syntax to tell it otherwise.

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

That has been traced down to a bug in lbr$_set_module on VAX/VMS 7.3. It was ignoring the size set in the input buffer and always writing 49 bytes.

In addition both the original and my modified code was incorrectly passing the address of the buffer descriptor length as the variable to get the actual length of the header. I did not experiment to see if the stack corruption went away when I fixed that.

The mhddef structure, which has the mhd$l_dattim member is sufficient.
If there is more data, the lbr$_set_module is supposed to truncate the data to the size given and return a status of LBR-W-HDRTRUNC which is 2525184. I can not find that value in any header file.

That is what was corrupting the stack on VAX.

Part of the fix is to allocate LBR$C_MAXHDRSIZ for the mhd structure and then free it after it is used.

Diagnosing this also exposed a bug in mhddef.h. The definition for mhd$l_datim is wrong. It has it as a 32 bit longword instead of a 64 bit structure or array.

If the mhddef.h was correct, then the decc$fix_time() would not have returned anything resembling a sane time.

Also if you run the test program with no input, a null pointer get referenced, so that also needs a check.

Not enough time to produce a diff tonight, but thought you would like to know why it was failing.

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"

Part of the problem was that some of the old names and comments were completely wrong, which would make comparing the new code to the old code difficult at best.

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

Unfortunately it was about the only way to do things the way the code is structured. Only the caller and the callback routine know what the structure of the argument is, and that structure needs some VMS specific data in it at those points.

I can look too see if macros can be added to hide the #ifdef VMS code to see if that could make it easier to read.

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 will look at that tomorrow.  It is late here.

It may be that a VMS programmers notes section of the readme is needed.

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.

I will look at simplifying it.

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.

I can look at that.  There are a lot of scripts that need it.

Also the perl scripts are not counting the test successes correctly on non-unix plaforms. I have not investigated how to fix that.

The perl script should report: Failed, Passed, Skipped, Expected Failed, and Unexpected Passed.

Expected failed and Unsexpected passed are for known bugs. Instead of just skipping a test that can not currently pass on platform, it should be marked expected fail. Then if someone fixes it, either on purpose or by accident, it will show up in the test run.

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

I currently can only run the Perl test script on the Alpha system. The symbol_vectors are not needed to test creating the library.

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.

I have not traced down why the perl touch method is not working on VMS
object files used for the librarian tests. All I know right now is that it does not work for me. Since I have a workaround, I am not investigating it.

I can try to make the comments more clear about that.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]