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

[h.becker]

On 06/06/2014 05:15 AM, John Malmberg wrote:
> Follow-up Comment #5, bug #41758 (project make):
> 
> Previous patch was not tested on VAX.  When tested on VAX this exposed a bug
> in the lbr$set_module() that needed to be worked around.
> 
> Small clean up of comments and code based on feedback from Hartmut Becker.
> 
> Copyright assignment is now on file with FSF for GNU Make.
> 
> (file #31514)
>     _______________________________________________________
> 
> Additional Item Attachment:
> 
> File name: 0001-Fix-archive-support-for-VMS-take-2.patch Size:29 KB

Couple of things...

lbr$set_module, the documentation says:

Buffer that receives the module header. The bufdesc argument is the
address of a string descriptor pointing to the buffer that receives the
module header. The buffer must be the size specified by the symbol
MHD$B_USRDAT plus the value of the CRE$L_UHDMAX create option. The MHD$
and CRE$ symbols are defined in the modules $MHDDEF and $CREDEF, which
are stored in SYS$LIBRARY:STARLET.MLB.

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.

and

Length of the module header. The buflen argument is the address of a
longword receiving the length of the returned module header.

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).

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.

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.