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

[John E. Malmberg]

On 6/10/2014 3:24 PM, h.becker wrote:
> On 06/06/2014 05:15 AM, John Malmberg wrote:
>
> lbr$set_module, the documentation says:
<snip>
> So the size of struct mhddef may not be enough and returning more than
> that is not an error. Allocating LBR$C_MAXHDRSIZ bytes for the buffer is
> probably the best one can do, because the contents of the fields
> mhd$b_usrdat and cre$l_uhd is not known, here.

This issue is only present on older versions of VMS, not the current 
version, so someone else also considered it a bug and fixed it.

The library routine is given a descriptor that sets the bounds that it 
can write to, and there is a return status code to indicate when there 
is more data to be read, so that the call can be repeated with a larger 
buffer.
> and
>
> Length of the module header. The buflen argument is the address of a
> longword receiving the length of the returned module header.

Will fix in the next patch submission.  That bug was present in the 
original code also.  This is why a prefer to hand construct the 
prototypes for the library routines and system services.  I can set them 
up to force a compile time error in these cases.

We are saved from data corruption because the compiler does not tightly 
pack variables so 32 bits was available for the write.  But this should 
get fixed.

> So in arscan.c
> "unsigned short buffer_length;  /* Actual buffer length */"
> should be changed to a type of "unsigned int".
>
> Regarding comments, I would like to have some comments like
>
> /* On VMS, (object) modules in libraries do not have suffixes. That is,
>     to find a match for a pattern, the pattern must not have any suffix.
>     So the suffix from the pattern is saved and the pattern is stripped
>     (ar_glob). If there is a match and the match, which is a module
>     name, is added to the chain, the saved suffix is added back to
>     construct a source filename (ar_glob_match). */
>
> maybe in "ar.c" just prior to "struct ar_glob_state". I admit, I don't
> know and didn't try to understand the code, so the above "constructed"
> comment may not be correct. (And it seems that this comment style is
> almost always used for block comments).

It is mostly correct and the next patch will have a similar comment.  I 
do not think it only applies to object modules though.

> In arscan.c, I suggest to use LBR$_HDRTRUNC instead of LIB_W_HDRTRUNC. A
> "globalvalue unsigned int LBR$_HDRTRUNC;" replaces the "#define
> LIB_W_HDRTRUNC 2525184" and the check changes to "if ((status !=
> LBR$_HDRTRUNC) && ..."
>
> Globalvalue is the right storage class for this declaration as per
> definition LBR$_HDRTRUNC can change any time.   With resolving it at link
> time and doing a fixup at activation time the code will get the correct
> value even if it is changed. I agree, I don't expect it to be changed,
> but having own #defines for VMS defined values looks wrong to me. And as
> far as I can see there are no LBR$_<code> return codes defined in any of
> the LBR header files and it seems unlikely that this will be changed
> (sooner or later or at all) by VMS engineering.

VMS convention has always been to use these as compile time constants 
which means that once they are set, they are never changed for backwards 
compatibility.

globalvalue is only visible if VAXC compatibility is active, and turning 
on stricter ANSI compliance in the compiler turns it off.  Turning on 
the stricter ANSI compliance finds bugs that VAX C compatibility 
ignores, so that is something that is desired.

While there is a ANSI compatible VMS extension to allow the equivalent 
of a globalvalue, it is a bit ugly to use.

And the compiler best optimizes as a constant, not as a global value 
that is not known until run time.

So with all that considered, just setting a constant missing from the 
header files seems the best solution.

As VAX C is only needed for building on VMS 5.5-1 and earlier, unless 
there is active builders for that old of VMS, I have been removing VAX C 
specific code as I update modules or converting it to ANSI compliance.

> In tests/scripts/vms/library I suggest to remove the "universal" option
> from the test script. For image libraries, librarian seems to be happy
> with an image which is flagged as a shareable image no matter whether it
> has a global symbol table or not. So there is no need to have one on VAX
> but none on Alpha/I64; in other words on all VMS platforms I suggest to
> either use a global symbol table or to use none.

I will look at doing that in a future patch.  It appears that the 
UNIVERSAL qualifier is ignored by non-VAX, so is not causing a problem.

Regards,
-John