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