Here's what I've discovered so far building the latest "blead" git sources of Perl on VMS x86-64 using the VSI C compiler (successfully) and then Clang (compiles but fails at startup).

I've forked the repo on GitHub and created my own branch for VMS fixes. My first set will be to fix various bugs in the Configure DCL script, as well as the Perl script that queries the git configuration when a ".git" directory is found (the VMS version of git doesn't recognize the commands given, and it generates a buggy .h file in a different way when "git" isn't found).

The default C build uses 32-bit pointers and 32-bit size_t on x86-64, so that's my baseline. For C++, I tried to build with 32-bit pointers, but ran into a brick wall with a compiler error when a "va_list *" couldn't be passed from a varargs function to a helper function due to pointer size mismatches. I assume this is one of the known issues with 32-bit pointers in Clang.

Without adding "/Pointer", I get compiler errors with argv[] being 32-bit pointers and getopt() expecting 64-bit pointers. So I have to explicitly use "/Pointer=64=argv". The first scary failure that made me concerned about the odds of Perl running as C++ is the "vms/munchconfig.c" helper program that's used to paste "config.sh" values into the generated config.h and descrip.mms files. The program opens two or three source files, passed on the command line (using getopt()), then reads from each in order, writing the results to stdout.

Munchconfig works perfectly when compiled as C, and I see no reason why it should fail when compiled as C++, but the first fgets() call to read the first line from the second opened file is failing, returning NULL, and causing a crash in the strstr() call. The program could definitely use better error handling, but the fgets() call shouldn't be failing.

FILE, on VMS, is a pointer to a struct _iosb, so a FILE * is a pointer to a pointer. I don't see how 32-bit or 64-bit pointers would make too much difference for such simple usage of stdio, and indeed the program crashes with either /Pointer=32 or /Pointer=64=argv. I'm concerned why C++ fails on such a simple program and don't see any obvious answers.

As a hack, I modified my Configure script to build munchconfig.exe with the C compiler to get past this crash, and that enabled me to find a few issues with 64-bit pointers, 64-bit long, and 64-bit size_t.

One mismatch that I had to add some code for was to handle the char * arrays returned by the DNS resolver functions, which are always 32 bits. For those cases, I added a hack for "#if defined(VMS) && PTRSIZE == 8" to malloc() an array of 64-bit char*, copy the pointers into the larger pointers, call the helper function, and then free() the new array. Not the most efficient solution, but this only gets called for hostname lookups so it's good enough.

Ideally, there should be 64-bit pointer versions of struct servent, struct hostent, etc., but that wasn't a huge issue for porting.

So far, I've had to fix several issues in vms/vms.c due to C++ cleanness with string functions (in C++, if you pass a const char * to e.g. strchr(), the return is a const char *, and if you pass a char *, the return value is, too, thanks to function overloading), and the use of "long int" to hold 32-bit VMS values, which I'm changing to "int", for maximum compatibility.

At present, I'm able to link miniperl.exe but it crashes in the VMS startup code at the second of the two _ckvmssts_noperl() calls at the start of vms_image_init() in vms/vms.c:

void
vms_image_init(int *argcp, char ***argvp)
{ ...
_ckvmssts_noperl(sys$getjpiw(0,NULL,NULL,jpilist,iosb,NULL,NULL));
_ckvmssts_noperl(iosb[0]);
...
}

In this case, jpilist is an itmlst_3, and iosb is "long int iosb[2]". I've tried removing the "long" everywhere in this file and that didn't seem to help, so now I'm going to have to go through more carefully and check that the proper struct alignment pragmas are being defined. I think I caught all the places that needed "|| defined(__clang__)" added to the __DECC and __DECCXX checks.

BTW, I realized it was a really bad idea for me to attempt to -D__DECCXX as a workaround for some code that tested for it. The VMS system header files really expect that not to be defined for Clang, which I figured out quickly.

I'll add to this thread as I have more info on why the Clang-compiled 64-bit pointer version is failing. If anyone at VSI can investigate the fgets() failure with multiple open input files (errno is EBADF, that's all I know), that would be excellent. I'll see if I can come up with a self-contained test case, if it would be helpful, and this isn't already a known issue.

Regards,
Jake Hamby

On 10/19/23 2:26 PM, Jake Hamby (Solid State Jake) wrote:

> Munchconfig works perfectly when compiled as C, and I see no reason
> why it should fail when compiled as C++, but the first fgets() call to
> read the first line from the second opened file is failing, returning
> NULL, and causing a crash in the strstr() call. The program could
> definitely use better error handling, but the fgets() call shouldn't be
> failing.

The good news is that munchconfig is a simple, standalone program that
does not require the rest of the Perl distribution or any other
dependencies to debug and test. The program is here:

https://raw.githubusercontent.com/Perl/perl5/blead/vms/munchconfig.c

It just takes key/value pairs from one file and uses them to do
substitutions on the second file:

$ type file1.txt
foo='define'
$ type file2.txt
!GROK!THIS!
#$foo HAS_FOO
!GROK!THIS!
$ mcr []munchconfig file1.txt file2.txt
#define HAS_FOO

On Thursday, October 19, 2023 at 2:34:19 PM UTC-7, Craig A. Berry wrote:
> On 10/19/23 2:26 PM, Jake Hamby (Solid State Jake) wrote:
>
> > Munchconfig works perfectly when compiled as C, and I see no reason
> > why it should fail when compiled as C++, but the first fgets() call to
> > read the first line from the second opened file is failing, returning
> > NULL, and causing a crash in the strstr() call. The program could
> > definitely use better error handling, but the fgets() call shouldn't be
> > failing.
> The good news is that munchconfig is a simple, standalone program that
> does not require the rest of the Perl distribution or any other
> dependencies to debug and test. The program is here:
>
> https://raw.githubusercontent.com/Perl/perl5/blead/vms/munchconfig.c
>
> It just takes key/value pairs from one file and uses them to do
> substitutions on the second file:
>
> $ type file1.txt
> foo='define'
> $ type file2.txt
> !GROK!THIS!
> #$foo HAS_FOO
> !GROK!THIS!
> $ mcr []munchconfig file1.txt file2.txt
> #define HAS_FOO

That does simplify things.

On Thursday, October 19, 2023 at 12:26:55 PM UTC-7, Jake Hamby wrote:
> Here's what I've discovered so far building the latest "blead" git sources of Perl on VMS x86-64 using the VSI C compiler (successfully) and then Clang (compiles but fails at startup).
>
> I've forked the repo on GitHub and created my own branch for VMS fixes. My first set will be to fix various bugs in the Configure DCL script, as well as the Perl script that queries the git configuration when a ".git" directory is found (the VMS version of git doesn't recognize the commands given, and it generates a buggy .h file in a different way when "git" isn't found).
>
> The default C build uses 32-bit pointers and 32-bit size_t on x86-64, so that's my baseline. For C++, I tried to build with 32-bit pointers, but ran into a brick wall with a compiler error when a "va_list *" couldn't be passed from a varargs function to a helper function due to pointer size mismatches. I assume this is one of the known issues with 32-bit pointers in Clang.
>
> Without adding "/Pointer", I get compiler errors with argv[] being 32-bit pointers and getopt() expecting 64-bit pointers. So I have to explicitly use "/Pointer=64=argv". The first scary failure that made me concerned about the odds of Perl running as C++ is the "vms/munchconfig.c" helper program that's used to paste "config.sh" values into the generated config.h and descrip.mms files. The program opens two or three source files, passed on the command line (using getopt()), then reads from each in order, writing the results to stdout.
>
> Munchconfig works perfectly when compiled as C, and I see no reason why it should fail when compiled as C++, but the first fgets() call to read the first line from the second opened file is failing, returning NULL, and causing a crash in the strstr() call. The program could definitely use better error handling, but the fgets() call shouldn't be failing.
>
> FILE, on VMS, is a pointer to a struct _iosb, so a FILE * is a pointer to a pointer. I don't see how 32-bit or 64-bit pointers would make too much difference for such simple usage of stdio, and indeed the program crashes with either /Pointer=32 or /Pointer=64=argv. I'm concerned why C++ fails on such a simple program and don't see any obvious answers.
>
> As a hack, I modified my Configure script to build munchconfig.exe with the C compiler to get past this crash, and that enabled me to find a few issues with 64-bit pointers, 64-bit long, and 64-bit size_t.
>
> One mismatch that I had to add some code for was to handle the char * arrays returned by the DNS resolver functions, which are always 32 bits. For those cases, I added a hack for "#if defined(VMS) && PTRSIZE == 8" to malloc() an array of 64-bit char*, copy the pointers into the larger pointers, call the helper function, and then free() the new array. Not the most efficient solution, but this only gets called for hostname lookups so it's good enough.
>
> Ideally, there should be 64-bit pointer versions of struct servent, struct hostent, etc., but that wasn't a huge issue for porting.
>
> So far, I've had to fix several issues in vms/vms.c due to C++ cleanness with string functions (in C++, if you pass a const char * to e.g. strchr(), the return is a const char *, and if you pass a char *, the return value is, too, thanks to function overloading), and the use of "long int" to hold 32-bit VMS values, which I'm changing to "int", for maximum compatibility.
>
> At present, I'm able to link miniperl.exe but it crashes in the VMS startup code at the second of the two _ckvmssts_noperl() calls at the start of vms_image_init() in vms/vms.c:
>
> void
> vms_image_init(int *argcp, char ***argvp)
> {
> ...
> _ckvmssts_noperl(sys$getjpiw(0,NULL,NULL,jpilist,iosb,NULL,NULL));
> _ckvmssts_noperl(iosb[0]);
> ...
> }
>
> In this case, jpilist is an itmlst_3, and iosb is "long int iosb[2]". I've tried removing the "long" everywhere in this file and that didn't seem to help, so now I'm going to have to go through more carefully and check that the proper struct alignment pragmas are being defined. I think I caught all the places that needed "|| defined(__clang__)" added to the __DECC and __DECCXX checks.
>
> BTW, I realized it was a really bad idea for me to attempt to -D__DECCXX as a workaround for some code that tested for it. The VMS system header files really expect that not to be defined for Clang, which I figured out quickly.
>
> I'll add to this thread as I have more info on why the Clang-compiled 64-bit pointer version is failing. If anyone at VSI can investigate the fgets() failure with multiple open input files (errno is EBADF, that's all I know), that would be excellent. I'll see if I can come up with a self-contained test case, if it would be helpful, and this isn't already a known issue.
>
> Regards,
> Jake Hamby

I've cleaned up my 64-bit and x86-64 patches and committed them to the "jhamby/vms-fixes" branch of my fork of Perl 5 on GitHub, which should also be the default branch:

https://github.com/jhamby/vms-perl5/

For testing purposes, I started from a clean tree and successfully built a 32-bit pointer version of Perl with the C compiler with these changes. After the test suite completes, I'm going to create a second tree for a "/Pointer=64=argv" build using the C compiler.

BTW, always use the "-q" option when checking out a git repo. Otherwise, the verbose status output creates a firestorm of interrupts if you're using the serial console, and I'm sure it's equally bad over ssh. Adding the "-q" option tremendously speeds up the clone.

Regards,
Jake Hamby

On 10/19/23 7:16 PM, Jake Hamby (Solid State Jake) wrote:
> On Thursday, October 19, 2023 at 2:34:19 PM UTC-7, Craig A. Berry wrote:
>> On 10/19/23 2:26 PM, Jake Hamby (Solid State Jake) wrote:
>>
>>> Munchconfig works perfectly when compiled as C, and I see no reason
>>> why it should fail when compiled as C++, but the first fgets() call to
>>> read the first line from the second opened file is failing, returning
>>> NULL, and causing a crash in the strstr() call. The program could
>>> definitely use better error handling, but the fgets() call shouldn't be
>>> failing.
>> The good news is that munchconfig is a simple, standalone program that
>> does not require the rest of the Perl distribution or any other
>> dependencies to debug and test. The program is here:
>>
>> https://raw.githubusercontent.com/Perl/perl5/blead/vms/munchconfig.c
>>
>> It just takes key/value pairs from one file and uses them to do
>> substitutions on the second file:
>>
>> $ type file1.txt
>> foo='define'
>> $ type file2.txt
>> !GROK!THIS!
>> #$foo HAS_FOO
>> !GROK!THIS!
>> $ mcr []munchconfig file1.txt file2.txt
>> #define HAS_FOO
>
> That does simplify things.

I finally got around to installing C++, which has a new version this
week. With this compiler:

$ cxx/vers
clang version 10.0.1 (git@bitbucket.org:vms_software/llvm-10.0.1.git
b2ed69f05fb4239b21b6ff2f806caaf4691c9719)
Build date: 09-19-2023
Target: x86_64-OpenVMS

on v9.2-1 with update 2.0 installed, I get a clean compile and link with:

$ cxx/pointer_size=64=argv/names=uppercase [.vms]munchconfig.c
$ link munchconfig

The /names=uppercase is necessary because decc$ga__optarg64 is an upper
case symbol in DECC$SHR.EXE but the compiler only creates a lower case
symbol. This is probably a header problem as I think Jake noted.

The result works absolutely fine for my trivial example:

$ type file1.txt
foo='define'
$ type file2.txt
!GROK!THIS!
#$foo HAS_FOO
!GROK!THIS!
$ mcr []munchconfig file1.txt file2.txt
#define HAS_FOO

However, for the real data, it blows up:

$ mcr []munchconfig config.sh config_h.sh
%SYSTEM-F-ACCVIO, access violation, reason mask=04, virtual
address=0000000000000000, PC=FFFF83000A040F85, PS=0000001B
%TRACE-F-TRACEBACK, symbolic stack dump follows
image module routine line rel PC abs PC
DECC$SHR STRLEN.C;1 #3645 00000000802D3F85
FFFF83000A040F85
DECC$SHR STRSTR.C;1 #3773 00000000802DA58C
FFFF83000A04758C
MUNCHCONFIG 0 000000008000173B
000000008000173B
MUNCHCONFIG 0 00000000800017BB
00000000800017BB
MUNCHCONFIG 0 00000000800009B9
00000000800009B9
MUNCHCONFIG 0 00000000800016A8
00000000800016A8
PTHREAD$RTL 0 000000008004122C
FFFF830009D0922C
PTHREAD$RTL 0 0000000080002316
FFFF830009CCA316
0 FFFF8300081FC0A6
FFFF8300081FC0A6
DCL 0 000000008006778B
000000007ADFF78B
%TRACE-I-LINENUMBER, Leading '#' specifies a source file record number.
%TRACE-I-END, end of TRACE stack dump

This is not actually a problem with fgets(), but just a simple buffer
overflow. One that apparently stomped on something that didn't matter
when built with the C compiler but caused trouble with C++. This fixes it:

$ gdiff -pu vms/munchconfig.c;-0 vms/munchconfig.c
--- vms/munchconfig.c;-0 2023-10-02 07:10:19 -0500
+++ vms/munchconfig.c 2023-10-20 15:04:13 -0500
@@ -32,7 +32,7 @@
#endif

/* The biggest line we can read in from a file */
-#define LINEBUFFERSIZE 1024
+#define LINEBUFFERSIZE 8192
#define NUMTILDESUBS 30
#define NUMCONFIGSUBS 1500
#define TOKENBUFFERSIZE 80
[end of patch]

On Friday, October 20, 2023 at 1:22:15 PM UTC-7, Craig A. Berry wrote:
>
> The /names=uppercase is necessary because decc$ga__optarg64 is an upper
> case symbol in DECC$SHR.EXE but the compiler only creates a lower case
> symbol. This is probably a header problem as I think Jake noted.

That explains that linker warning.

> However, for the real data, it blows up:
>
> This is not actually a problem with fgets(), but just a simple buffer
> overflow. One that apparently stomped on something that didn't matter
> when built with the C compiler but caused trouble with C++. This fixes it:
>
> $ gdiff -pu vms/munchconfig.c;-0 vms/munchconfig.c
> --- vms/munchconfig.c;-0 2023-10-02 07:10:19 -0500
> +++ vms/munchconfig.c 2023-10-20 15:04:13 -0500
> @@ -32,7 +32,7 @@
> #endif
>
> /* The biggest line we can read in from a file */
> -#define LINEBUFFERSIZE 1024
> +#define LINEBUFFERSIZE 8192
> #define NUMTILDESUBS 30
> #define NUMCONFIGSUBS 1500
> #define TOKENBUFFERSIZE 80
> [end of patch]

Thank you so much for sorting that out! I would never have suspected a buffer overflow on a 1024-byte line buffer.

After patching those two issues locally, the "munchconfig" utility works and runs fine for me as C++ as well.

Regards,
Jake Hamby

Thanks to Craig Berry for figuring out that the "munchconfig" program was failing as C++ due to a buffer overflow. In the meantime, I was able to resolve some of the 32-bit / 64-bit pointer mismatches in vms/vms.c and now "miniperl" isn't crashing but instead is getting confused by some VMS-style filenames and trying to double-escape everything with "^".

vms/vms.c is a single 14000 line file, so it's a bit of a monster to work on. I'm taking my usual VMS code modernization approach of adding:

#define __NEW_STARLET 1

before including the header files, to pick up the function prototypes. With __NEW_STARLET defined, I'm getting a lot of build errors in vms/vms.c that I'm sorting through now. In addition, I changed all the descriptors and item lists from 32-bit to 64-bit versions, except for one case where it's calling an undocumented DECwindows callback, which may not work on x86-64 anyway due to having to comment out a call to unimplemented VAXC$ESTABLISH.

I remember now why I wasn't able to get very far on 64-bit-pointer-ifying open-source programs on Alpha and Itanium. Having size_t and ptrdiff_t defined as 32-bit, while pointers are 64 bits, causes too many build failures in a program like Perl to want to attempt to fix. I'm glad the Clang 64-bit pointer ABI is using 64-bit size_t and 64-bit long as well (LP64). I had to give up trying to build Perl with VSI C and 64-bit pointers because of the 32-bit size_t.

Even though changing "long" from 32 to 64 bits creates some bugs in the short term, it's the better option for avoiding problems such as time_t overflowing in 2038, and other variable types that expand to 64 bits if they were defined in terms of the "long" type. Every major 64-bit OS that I know of, except Windows, is "LP64" (64-bit longs and pointers). Windows uses 32-bit long and 64-bit "long long", or "LLP64".

Here are a few miscellaneous notes about VMS APIs and Perl's use of them:

- sys$hash_password returns a quadword, not a string. "Perl_my_crypt()" treats the returned value as a string, which it's not.

- sys$forcex annoyingly requires a 32-bit pointer reference to the pid to use. I'm working around this by making the struct that contains the pid a 32-bit pointer struct, which works out well since the code is calling lib$get_vm() to allocate that struct, instead of the normal Perl malloc() wrapper.

- I see a few mismatches between expecting short filenames and long filenames (255 or 4095 chars, plus null byte). Perl defines VMS_MAXRSS as the max filespec length including '\0', so that should be used more consistently in the code to avoid hard-to-detect buffer overflows. How many people are really using paths longer than 256 characters in practice?

- I need to verify to make sure lib$initialize() is being called from C++, in case that has anything to do with unexpected filename mangling

- There's a use of sys$change_acl for attempting to delete a file that I need to change to reuse code from an updated implementation of rename() that uses sys$get_security and sys$set_security. There's a code comment about sys$change_acl being undocumented for a long time, and when I build with __NEW_STARLET, it gives an error that it expects 9 arguments instead of 7. It's better to refactor the remove and rename code to use the same helper functions to add/remove the "give-myself-all-privs" ACL from the file to try to rename or delete when the first try doesn't work.

I think that's it for now. At this point, I can hardly wait to see what happens after I clean up the VMS code to use the correct and most 64-bit-capable APIs. None of the changes I've made should affect Alpha, Itanium, or 32-bit pointer versions, but I'll be sure to test that before committing my changes.

Regards,
Jake Hamby

On Saturday, October 21, 2023 at 1:21:11 AM UTC+2, Jake Hamby (Solid State Jake) wrote:
....
> - I need to verify to make sure lib$initialize() is being called from C++, in case that has anything to do with unexpected filename mangling

Lib$initialize isn't called by C++. It is a "Don't call us, we call you." type of thing. For your image, you set up an array of function pointers (aka init array). With some linker magic, VMS will know and find this array and call each of the functions from this array - before any (non-init) code of this image is executed. This array must contain 32-bit pointers. Any 32-bit zero in the array is seen as the end of the list of functions to call.

On x86, if you compile all C code with C++, by default the array will be an array of 64 bit pointers. Because taking the address of a function results in a 32-bit address, the VMS magic will see and call only the first function of this array. If Perl has more than one (64-bit) function pointer in this array, some init code is not called and this may explain why there is some "unexpected filename mangling".

The usual approach to get 32 bit pointers is to wrap the definition of the array with
#if __INITIAL_POINTER_SIZE
# pragma __pointer_size __save
# pragma __pointer_size 32
#endif
and
#if __INITIAL_POINTER_SIZE
# pragma __pointer_size __restore
#endif

This may be in the Perl module(s) defining the init array. However, by default the C++ compiler on x86 does not set the __INITIAL_POINTER_SIZE macro. It is only set if the /POINTER_SIZE qualifier is present. For compiling with C++ and the above wrappers to work you have to compile with /POINTER_SIZE=64. (Or, conditionalize the code for C++ on x86 instead of __INITIAL_POINTER_SIZE, or ...)

And then there is the other feature of C++, that it's not enough to have the image initialization in the main image to get all the C RTL features set. At least DECC$ARGV_PARSE_STYLE has to be set before the C++ RTLs are initialized. How to do this was already discussed here in cov.

On 10/21/23 6:38 AM, hb@end.of.inter.net wrote:
> On Saturday, October 21, 2023 at 1:21:11 AM UTC+2, Jake Hamby (Solid State Jake) wrote:
> ...
>> - I need to verify to make sure lib$initialize() is being called from C++, in case that has anything to do with unexpected filename mangling
>
>> Lib$initialize isn't called by C++. It is a "Don't call us, we call you." type of thing. For your image, you set up an array of function pointers (aka init array). With some linker magic, VMS will know and find this array and call each of the functions from this array - before any (non-init) code of this image is executed. This array must contain 32-bit pointers. Any 32-bit zero in the array is seen as the end of the list of functions to call.
>>
>> On x86, if you compile all C code with C++, by default the array will be an array of 64 bit pointers. Because taking the address of a function results in a 32-bit address, the VMS magic will see and call only the first function of this array. If Perl has more than one (64-bit) function pointer in this array, some init code is not called and this may explain why there is some "unexpected filename mangling".
>>
> The usual approach to get 32 bit pointers is to wrap the definition of the array with
> #if __INITIAL_POINTER_SIZE
> # pragma __pointer_size __save
> # pragma __pointer_size 32
> #endif
> and
> #if __INITIAL_POINTER_SIZE
> # pragma __pointer_size __restore
> #endif
>
> This may be in the Perl module(s) defining the init array.

It is:

https://github.com/Perl/perl5/blob/blead/vms/vms.c#L14068

> However, by default the C++ compiler on x86 does not set the __INITIAL_POINTER_SIZE macro. It is only set if the /POINTER_SIZE qualifier is present. For compiling with C++ and the above wrappers to work you have to compile with /POINTER_SIZE=64. (Or, conditionalize the code for C++ on x86 instead of __INITIAL_POINTER_SIZE, or ...)
>

Jake appears to be using /Pointer=64=argv:

https://github.com/jhamby/vms-perl5/commit/191a621d35c9edf5a5f7026a3c7e10b4a7f5c3fb#diff-7fd27421e313a9bec5978be2439473d4b1c22df909e64db60851e0aee4c2251fR1579

It seems bizarre to me that in order to be able to override the default
pointer size with a pragma, you have to specify the pointer size on the
command line that is already the default. But that's what the release
notes say.

> And then there is the other feature of C++, that it's not enough to have the image initialization in the main image to get all the C RTL features set. At least DECC$ARGV_PARSE_STYLE has to be set before the C++ RTLs are initialized. How to do this was already discussed here in cov.

It's more of a bug than a feature.

Almost all of the Perl core is in its own shareable image that gets
loaded at run time using LIB$FIND_IMAGE_SYMBOL. But it also, early in
the build, links the whole shebang as a static image that is used to
complete the build and run the test suite. Neither one of these hits the
LIB$INITIALIZE code early enough for DECC$ARGV_PARSE_STYLE when built
with C++. This architecture is the same across platforms. Changing it
would be extra complexity and fragility, which are generally bad things
in software engineering.

On 10/19/23 2:26 PM, Jake Hamby (Solid State Jake) wrote:
> Here's what I've discovered so far building the latest "blead" git
> sources of Perl on VMS x86-64 using the VSI C compiler (successfully)
> and then Clang (compiles but fails at startup).
>
> I've forked the repo on GitHub and created my own branch for VMS
> fixes.

Thanks for working on this. I've been looking through the code at:

https://github.com/jhamby/vms-perl5/commits/jhamby/vms-fixes#

There is some really good stuff here, but I have some comments.

First of all, if you ever want any of this to be accepted upstream, the
commits need to be more atomic. For example, this one:

https://github.com/jhamby/vms-perl5/commit/191a621d35c9edf5a5f7026a3c7e10b4a7f5c3fb

has 13 bullet points in the commit message and addresses 13 separate and
independent things, some of which have nothing to do with configure.com
and thus belie the first line message "Multiple fixes to VMS configure
script."

Workarounds due to compilers in field test should not be hard-coded as
configuration options based on platform. For example, saying that the
long double size is 64 bits is just a little hack while waiting for long
doubles to be implemented. For now it's better to just configure with
"-"Duser_c_flags=/L_DOUBLE=64" and wait for the compiler to be
completed. Ditto for infinities, lib$establish, etc.

In some cases it's hard to know what will eventually be supported. For
example, the networking structs like servent and hostent are part of the
networking stack and not the CRTL, which may be why there are no 64-bit
versions. Will that stuff eventually be made 64-bit clean? I don't know.

The pointer size should not be hard-coded in the compiler flags by
platform. The -Duse64bitall configuration option should be the default
for OpenVMS x86 and the pointer size should follow that (and *that*
might need to be platform-specific).

There are a lot of places where you are changing something like this:

#if defined(__DECC) || defined(__DECCXX)

with

#if defined(__DECC) || defined(__DECCXX) || defined(__clang__)

That probably does little if any harm, especially if it's only done in
vms/vms.c, but it does change us from a check that is only meaningful on
VMS (or Tru64) to a check that could be meaningful elsewhere. The
reason for this code is that once upon a time there was a GCC that ran
on VMS but did not support various pragmas and other VMS-specific
features. Ideally we would test for the feature rather than the
compiler. I've looked at the macros defined for clang++ on VMS and
don't see anything obvious that looks better. One can collect all of the
macros defined by clang++ by doing:

$ clang -"dM" -"E" -"x" "c++" tt

New code that is VMS-specific should really check for "__VMS" rather
than "VMS". Both still work, and this is kind of a pedantic point, but
new things should probably be done the "new" way, i.e., how it should
have been done in recent decades. It's always seemed like too much
churn to go back and change all the existing ones.

On Saturday, October 21, 2023 at 12:52:06 PM UTC-7, Craig A. Berry wrote:
> On 10/19/23 2:26 PM, Jake Hamby (Solid State Jake) wrote:
> > Here's what I've discovered so far building the latest "blead" git
> > sources of Perl on VMS x86-64 using the VSI C compiler (successfully)
> > and then Clang (compiles but fails at startup).
> >
> > I've forked the repo on GitHub and created my own branch for VMS
> > fixes.
> Thanks for working on this. I've been looking through the code at:
>
> https://github.com/jhamby/vms-perl5/commits/jhamby/vms-fixes#
>
> There is some really good stuff here, but I have some comments.
>
> First of all, if you ever want any of this to be accepted upstream, the
> commits need to be more atomic. For example, this one:
>
> https://github.com/jhamby/vms-perl5/commit/191a621d35c9edf5a5f7026a3c7e10b4a7f5c3fb
>
> has 13 bullet points in the commit message and addresses 13 separate and
> independent things, some of which have nothing to do with configure.com
> and thus belie the first line message "Multiple fixes to VMS configure
> script."
>
> Workarounds due to compilers in field test should not be hard-coded as
> configuration options based on platform. For example, saying that the
> long double size is 64 bits is just a little hack while waiting for long
> doubles to be implemented. For now it's better to just configure with
> "-"Duser_c_flags=/L_DOUBLE=64" and wait for the compiler to be
> completed. Ditto for infinities, lib$establish, etc.
>
> In some cases it's hard to know what will eventually be supported. For
> example, the networking structs like servent and hostent are part of the
> networking stack and not the CRTL, which may be why there are no 64-bit
> versions. Will that stuff eventually be made 64-bit clean? I don't know.
>
> The pointer size should not be hard-coded in the compiler flags by
> platform. The -Duse64bitall configuration option should be the default
> for OpenVMS x86 and the pointer size should follow that (and *that*
> might need to be platform-specific).
>
> There are a lot of places where you are changing something like this:
>
> #if defined(__DECC) || defined(__DECCXX)
>
> with
>
> #if defined(__DECC) || defined(__DECCXX) || defined(__clang__)
>
> That probably does little if any harm, especially if it's only done in
> vms/vms.c, but it does change us from a check that is only meaningful on
> VMS (or Tru64) to a check that could be meaningful elsewhere. The
> reason for this code is that once upon a time there was a GCC that ran
> on VMS but did not support various pragmas and other VMS-specific
> features. Ideally we would test for the feature rather than the
> compiler. I've looked at the macros defined for clang++ on VMS and
> don't see anything obvious that looks better. One can collect all of the
> macros defined by clang++ by doing:
>
> $ clang -"dM" -"E" -"x" "c++" tt
>
> New code that is VMS-specific should really check for "__VMS" rather
> than "VMS". Both still work, and this is kind of a pedantic point, but
> new things should probably be done the "new" way, i.e., how it should
> have been done in recent decades. It's always seemed like too much
> churn to go back and change all the existing ones.

Thanks for the feedback. Once I get everything working, maybe I'll make a new branch with more atomic commits if it will help to upstream changes.

I just finished making a massive change to "vms/vms.c" to convert all the descriptors and item lists from 32-bit to 64-bit. Does it make a difference? I don't know yet because now my program is crashing very early in startup attempting to set a logical name, and I don't know why.

At least I know that the early startup code in vmsperl_set_features() is running, because that's where it crashes. I've split out the create logical code into its own test program, and I have no clue why it's failing with "%SYSTEM-F-NOLOGTAB, no logical name table name match". I made a version with 32-bit descriptors and item lists, and it fails. It fails with the C compiler as well. Is there something simple I'm forgetting?

#define __NEW_STARLET 1

#include <descrip.h>
#include <iledef.h>
#include <lib$routines.h>
#include <lnmdef.h>
#include <namdef.h>
#include <psldef.h>
#include <ssdef.h>
#include <starlet.h>
#include <strdef.h>
#include <stsdef.h>
#include <stdlib.h>
#include <string.h>

int main() {
static $DESCRIPTOR64(d_tab, (char *) "LNM$PROCESS");
const char *name = "DECC$EFS_CASE_PRESERVE";
const char *eqv = "ENABLE";

struct dsc64$descriptor_s d_name = {1, DSC$K_DTYPE_T, DSC$K_CLASS_S, -1, 0, NULL};
unsigned int iss, attr = LNM$M_CONFINE;
unsigned char acmode = PSL$C_USER;

struct _ileb_64 lnmlst[2] = {{1, LNM$_STRING, -1, 0, NULL, NULL},
{0, 0, 0, 0, NULL, NULL}};

d_name.dsc64$q_length = strlen(name);
d_name.dsc64$pq_pointer = (char *)name; /* Cast OK for read only parameter */

lnmlst[0].ileb_64$q_length = strlen(eqv);
lnmlst[0].ileb_64$pq_bufaddr = (char *)eqv; /* Cast OK for read only parameter */

iss = sys$crelnm(&attr, &d_tab, &d_name, &acmode, lnmlst);
if (!$VMS_STATUS_SUCCESS(iss)) lib$signal(iss);
}

Regards,
Jake Hamby

On Saturday, October 21, 2023 at 4:32:45 PM UTC-7, h47...@gmail.com wrote:
> On Sunday, October 22, 2023 at 12:26:01 AM UTC+2, Jake Hamby (Solid State Jake) wrote:
> ...
> > int main() {
> > static $DESCRIPTOR64(d_tab, (char *) "LNM$PROCESS");
> The string length in the descriptor is wrong. Here, sizeof ((char*)-1 is used , sizeof "LNM$PROCESS"-1 is the correct length. This can be seen in the debugger.
> static $DESCRIPTOR64(d_tab, "LNM$PROCESS");
> is what you want/need.

Thanks for the quick response! Removing the "(char *)" cast does indeed fix the issue. The reason I added it in the first place was to avoid this C++ warning:

DKA0:[local]test_crelnm_simple.c;12:17:33: warning: conversion from string literal to 'char * __ptr64' is deprecated [-Wc++11-compat-deprecated-writable-strings]
static $DESCRIPTOR64(d_tab, "LNM$PROCESS");
^
I didn't take into account that casting a const char array to a char * would change the behavior of sizeof() to return the size of the pointer and not the size of the NULL-terminated string! Since C and C++ won't allow a cast to "char[]", does anyone know of a way to get rid of the warning without breaking the macro?

Regards,
Jake Hamby

On 10/21/2023 8:09 PM, Jake Hamby (Solid State Jake) wrote:
> On Saturday, October 21, 2023 at 4:32:45 PM UTC-7, h47...@gmail.com wrote:
>> On Sunday, October 22, 2023 at 12:26:01 AM UTC+2, Jake Hamby (Solid State Jake) wrote:
>> ...
>>> int main() {
>>> static $DESCRIPTOR64(d_tab, (char *) "LNM$PROCESS");
>> The string length in the descriptor is wrong. Here, sizeof ((char*)-1 is used , sizeof "LNM$PROCESS"-1 is the correct length. This can be seen in the debugger.
>> static $DESCRIPTOR64(d_tab, "LNM$PROCESS");
>> is what you want/need.
>
> Thanks for the quick response! Removing the "(char *)" cast does indeed fix the issue. The reason I added it in the first place was to avoid this C++ warning:
>
> DKA0:[local]test_crelnm_simple.c;12:17:33: warning: conversion from string literal to 'char * __ptr64' is deprecated [-Wc++11-compat-deprecated-writable-strings]
> static $DESCRIPTOR64(d_tab, "LNM$PROCESS");
> ^
> I didn't take into account that casting a const char array to a char * would change the behavior of sizeof() to return the size of the pointer and not the size of the NULL-terminated string! Since C and C++ won't allow a cast to "char[]", does anyone know of a way to get rid of the warning without breaking the macro?

$DESCRIPTOR (and I assume its big brother $DESCRIPTOR64) are
convenient for lot of adhoc programming, but often I end up
switching to manually setting up the struct. One of the
reasons is to want to use strlen instead of sizeof.

Arne

On 10/20/2023 7:21 PM, Jake Hamby (Solid State Jake) wrote:
> Even though changing "long" from 32 to 64 bits creates some bugs in
> the short term, it's the better option for avoiding problems such as
> time_t overflowing in 2038, and other variable types that expand to
> 64 bits if they were defined in terms of the "long" type.

time_t is not defined in terms of long in the C standard.

You can have a 32 bit long and a 64 bit time_t. And
that is not a theoretical scenario - it is like
that on 64 bit Windows. And on VMS x86-64 with C++
it is the other way around: long is 64 bit and
time_t is 32 bit.

So size of long does not really impact the 32 bit
time_t problem.

Arne

PS: VMS does not have a 2038 problem - it has a 2106
problem - time_t is unsigned 32 bit on VMS.

On 10/21/23 7:19 PM, Arne Vajhøj wrote:
> On 10/21/2023 8:09 PM, Jake Hamby (Solid State Jake) wrote:
>> On Saturday, October 21, 2023 at 4:32:45 PM UTC-7, h47...@gmail.com
>> wrote:
>>> On Sunday, October 22, 2023 at 12:26:01 AM UTC+2, Jake Hamby (Solid
>>> State Jake) wrote:
>>> ...
>>>> int main() {
>>>> static $DESCRIPTOR64(d_tab, (char *) "LNM$PROCESS");
>>> The string length in the descriptor is wrong. Here, sizeof ((char*)-1
>>> is used , sizeof "LNM$PROCESS"-1 is the correct length. This can be
>>> seen in the debugger.
>>> static $DESCRIPTOR64(d_tab, "LNM$PROCESS");
>>> is what you want/need.
>>
>> Thanks for the quick response! Removing the "(char *)" cast does
>> indeed fix the issue. The reason I added it in the first place was to
>> avoid this C++ warning:
>>
>> DKA0:[local]test_crelnm_simple.c;12:17:33: warning: conversion from
>> string literal to 'char * __ptr64' is deprecated
>> [-Wc++11-compat-deprecated-writable-strings]
>>      static $DESCRIPTOR64(d_tab, "LNM$PROCESS");
>>                                  ^
>> I didn't take into account that casting a const char array to a char *
>> would change the behavior of sizeof() to return the size of the
>> pointer and not the size of the NULL-terminated string! Since C and
>> C++ won't allow a cast to "char[]", does anyone know of a way to get
>> rid of the warning without breaking the macro?
>
> $DESCRIPTOR (and I assume its big brother $DESCRIPTOR64) are
> convenient for lot of adhoc programming, but often I end up
> switching to manually setting up the struct. One of the
> reasons is to want to use strlen instead of sizeof.

And manually setting up the struct as follows seems to work in this case:

--- crelnm64.c;-0 2023-10-21 17:58:12 -0500
+++ crelnm64.c 2023-10-21 20:37:24 -0500
@@ -15,7 +15,7 @@
#include <string.h>

int main() {
- static $DESCRIPTOR64(d_tab, (char *) "LNM$PROCESS");
+ struct dsc64$descriptor_s d_tab = {1, DSC$K_DTYPE_T,
DSC$K_CLASS_S, -1, sizeof("LNM$PROCESS")-1, "LNM$PROCESS"};
const char *name = "DECC$EFS_CASE_PRESERVE";
const char *eqv = "ENABLE";

[end of patch]

On 10/21/2023 8:19 PM, Arne Vajhøj wrote:
> On 10/21/2023 8:09 PM, Jake Hamby (Solid State Jake) wrote:
>> On Saturday, October 21, 2023 at 4:32:45 PM UTC-7, h47...@gmail.com
>> wrote:
>>> On Sunday, October 22, 2023 at 12:26:01 AM UTC+2, Jake Hamby (Solid
>>> State Jake) wrote:
>>> ...
>>>> int main() {
>>>> static $DESCRIPTOR64(d_tab, (char *) "LNM$PROCESS");
>>> The string length in the descriptor is wrong. Here, sizeof ((char*)-1
>>> is used , sizeof "LNM$PROCESS"-1 is the correct length. This can be
>>> seen in the debugger.
>>> static $DESCRIPTOR64(d_tab, "LNM$PROCESS");
>>> is what you want/need.
>>
>> Thanks for the quick response! Removing the "(char *)" cast does
>> indeed fix the issue. The reason I added it in the first place was to
>> avoid this C++ warning:
>>
>> DKA0:[local]test_crelnm_simple.c;12:17:33: warning: conversion from
>> string literal to 'char * __ptr64' is deprecated
>> [-Wc++11-compat-deprecated-writable-strings]
>>      static $DESCRIPTOR64(d_tab, "LNM$PROCESS");
>>                                  ^
>> I didn't take into account that casting a const char array to a char *
>> would change the behavior of sizeof() to return the size of the
>> pointer and not the size of the NULL-terminated string! Since C and
>> C++ won't allow a cast to "char[]", does anyone know of a way to get
>> rid of the warning without breaking the macro?
>
> $DESCRIPTOR (and I assume its big brother $DESCRIPTOR64) are
> convenient for lot of adhoc programming, but often I end up
> switching to manually setting up the struct. One of the
> reasons is to want to use strlen instead of sizeof.

Here is an example:

$ type dfun.c
#include <stdio.h>

#include <descrip.h>

#define s1 "This is a test"
char s2[] = "This is a test";
char *s3 = "This is a test";
char s4[100] = "This is a test";

int main(int argc, char *argv[])
{ $DESCRIPTOR(d1, s1);
printf("str = %s, len = %d\n", d1.dsc$a_pointer, d1.dsc$w_length);
$DESCRIPTOR(d2, s2);
printf("str = %s, len = %d\n", d2.dsc$a_pointer, d2.dsc$w_length);
$DESCRIPTOR(d3, s3);
printf("str = %s, len = %d\n", d3.dsc$a_pointer, d3.dsc$w_length);
$DESCRIPTOR(d4, s4);
printf("str = %s, len = %d\n", d4.dsc$a_pointer, d4.dsc$w_length);
return 0;
} $ cc dfun
$ lin dfun
$ r dfun
str = This is a test, len = 14
str = This is a test, len = 14
str = This is a test, len = 3
str = This is a test, len = 99

Arne

On 10/21/2023 9:40 PM, Craig A. Berry wrote:
> On 10/21/23 7:19 PM, Arne Vajhøj wrote:
>> On 10/21/2023 8:09 PM, Jake Hamby (Solid State Jake) wrote:
>>> I didn't take into account that casting a const char array to a char
>>> * would change the behavior of sizeof() to return the size of the
>>> pointer and not the size of the NULL-terminated string! Since C and
>>> C++ won't allow a cast to "char[]", does anyone know of a way to get
>>> rid of the warning without breaking the macro?
>>
>> $DESCRIPTOR (and I assume its big brother $DESCRIPTOR64) are
>> convenient for lot of adhoc programming, but often I end up
>> switching to manually setting up the struct. One of the
>> reasons is to want to use strlen instead of sizeof.
>
> And manually setting up the struct as follows seems to work in this case:
>
> --- crelnm64.c;-0    2023-10-21 17:58:12 -0500
> +++ crelnm64.c    2023-10-21 20:37:24 -0500
> @@ -15,7 +15,7 @@
>  #include <string.h>
>
>  int main() {
> -    static $DESCRIPTOR64(d_tab, (char *) "LNM$PROCESS");
> +    struct dsc64$descriptor_s d_tab = {1,  DSC$K_DTYPE_T,
> DSC$K_CLASS_S, -1, sizeof("LNM$PROCESS")-1, "LNM$PROCESS"};
>      const char *name = "DECC$EFS_CASE_PRESERVE";
>      const char *eqv = "ENABLE";
>
> [end of patch]

I would feel tempted to use:

char *tab = "LNM$PROCESS";
struct dsc64$descriptor_s d_tab = {1, DSC$K_DTYPE_T, DSC$K_CLASS_S, -1,
strlen(tab), tab};

Only one place to change the name and less risk to mess
up if one need to change from a constant string to a
pointer like argv[1].

Arne

On 10/21/23 8:52 PM, Arne Vajhøj wrote:
> On 10/21/2023 9:40 PM, Craig A. Berry wrote:
>> On 10/21/23 7:19 PM, Arne Vajhøj wrote:
>>> On 10/21/2023 8:09 PM, Jake Hamby (Solid State Jake) wrote:
>>>> I didn't take into account that casting a const char array to a char
>>>> * would change the behavior of sizeof() to return the size of the
>>>> pointer and not the size of the NULL-terminated string! Since C and
>>>> C++ won't allow a cast to "char[]", does anyone know of a way to get
>>>> rid of the warning without breaking the macro?
>>>
>>> $DESCRIPTOR (and I assume its big brother $DESCRIPTOR64) are
>>> convenient for lot of adhoc programming, but often I end up
>>> switching to manually setting up the struct. One of the
>>> reasons is to want to use strlen instead of sizeof.
>>
>> And manually setting up the struct as follows seems to work in this case:
>>
>> --- crelnm64.c;-0    2023-10-21 17:58:12 -0500
>> +++ crelnm64.c    2023-10-21 20:37:24 -0500
>> @@ -15,7 +15,7 @@
>>   #include <string.h>
>>
>>   int main() {
>> -    static $DESCRIPTOR64(d_tab, (char *) "LNM$PROCESS");
>> +    struct dsc64$descriptor_s d_tab = {1,  DSC$K_DTYPE_T,
>> DSC$K_CLASS_S, -1, sizeof("LNM$PROCESS")-1, "LNM$PROCESS"};
>>       const char *name = "DECC$EFS_CASE_PRESERVE";
>>       const char *eqv = "ENABLE";
>>
>> [end of patch]
>
> I would feel tempted to use:
>
> char *tab = "LNM$PROCESS";
> struct dsc64$descriptor_s d_tab = {1,  DSC$K_DTYPE_T, DSC$K_CLASS_S, -1,
> strlen(tab), tab};
>
> Only one place to change the name and less risk to mess
> up if one need to change from a constant string to a
> pointer like argv[1].

That does look better, but unfortunately while these suggestions work
for C, there is a warning with clang++:

$ cxx/pointer_size=64=argv crelnm64.c/noopt/debug
DKA0:[CRAIG.test]crelnm64.c;9:18:17: warning: conversion from string
literal to 'char * __ptr64' is deprecated
[-Wc++11-compat-deprecated-writable
-strings]
char *tab = "LNM$PROCESS";
^
1 warning generated.

On Saturday, October 21, 2023 at 7:35:04 PM UTC-7, Craig A. Berry wrote:
> On 10/21/23 8:52 PM, Arne Vajhøj wrote:
> > Only one place to change the name and less risk to mess
> > up if one need to change from a constant string to a
> > pointer like argv[1].
> That does look better, but unfortunately while these suggestions work
> for C, there is a warning with clang++:
>
> $ cxx/pointer_size=64=argv crelnm64.c/noopt/debug
> DKA0:[CRAIG.test]crelnm64.c;9:18:17: warning: conversion from string
> literal to 'char * __ptr64' is deprecated
> [-Wc++11-compat-deprecated-writable
> -strings]
> char *tab = "LNM$PROCESS";
> ^
> 1 warning generated.

Yes, I ran into that and then discovered that attempting to add compiler flags containing "" in the configure script was a recipe for disaster. I tried all sorts of combinations of double, triple, and quadruple quoting, but I couldn't get it to work properly because DCL kept trying to interpret parts of the command line, so the script failed complaining about "C++11" or "COMPAT" not found. And the "++" in the warning message means it has to be quoted to work.

/warn=(disable="c++11-compat-deprecated-writable-strings")

I ended up adding the qualifier to my vms/descrip_mms.template file but then I realized that it's not strictly necessary, because Clang, unlike the C compiler, doesn't insert anything in the .obj file to indicate that compiler warnings were found, which is a big problem with the C compiler because MMK will abort on warnings, including the linker warning that some of the object files had warnings. So it's more important to ensure that the C version continues to compile without warnings than that the C++ version has a few (mostly having to do with a few functions in the non-VMS perl sources that don't always return a value from every code path. You'd think that'd be a compiler error, but apparently not).

Rather than commit a change to vms/vms.c that touches most of the lines in the file by changing *everything* to 64-bit structs, which nobody will want to review or merge, I made a copy of my 64-bit version and reverted back to the top-of-tree in my branch. Then I added back the #define __NEW_STARLET 1, and now I'm fixing the compiler errors by adding casts and removing "const" where necessary, while keeping the original 32-bit descriptors and item lists.

One reason I thought it might be necessary to change all of the descriptors and item lists to 64-bit is that I'm passing a lot of local variables and pointers into these system services, and the compiler only complains about "unsigned int" vs. "int", or a "const" pointer being assigned or passed to a non-const, but the "__ptr32" in the warnings made it seem at first as if the problem was casting a long pointer to a short one:

SYS$COMMON:[SYSLIB]SYS$STARLET_C.TLB;1(STARLET):4405:5: note: candidate function not viable: no known conversion from 'int *' to 'unsigned int * __ptr32' for 1st argument

Changing the type to "unsigned int" makes the warning disappear. Does this mean that the default behavior of Clang is to truncate long pointers to short ones silently? Or does it somehow know that local variables on the stack and global variables are in 32-bit space? In that case, what about malloc()'d memory? Something doesn't quite add up here.

After I sorted out the descriptor definitions, my all-64-bit version was crashing very early in the VMS-to-UNIX filename translation code in vms.c, and that's one reason I thought it'd be good to take a step back and try cleaning up the original code in smaller pieces. I'll still likely commit a big change to my tree with multiple fixes, and then I'll make a new branch for patches to submit pull requests for, so I can split things up into individual bug fixes once I know I haven't broken anything.

One potential bug appears to be in pipe_mbxtofd_ast(), where a "\n" is added to the received data in the passed "Pipe" struct pointer, before write()'ing it to the output file descriptor in the same struct. The reason I suspect this is causing issues is a few Perl test cases fail expecting "ok" and getting "\nok".

There are a few other single-line fixes that I can turn into separate pull requests. Clang warns about a char being compared to ">= 0x80" always being false, so I had to cast that to (unsigned char) before the comparison; the code in copy_expand_unix_filename_escape() does this in most cases, but not the first if() statement. One or two other issues caught by the compiler could be bugs due to not properly taking the address of the value to pass..

One slightly larger refactoring that I made in my all-64-bit version that I'll definitely commit after the commits to clean everything up to build with C++ and __NEW_STARLET prototypes, is to separate out the code to temporarily add an ACL giving full control to a file after a rename or delete fails, to see if it succeeds with the ACL (which is then removed after the rename, or if the delete fails). The vms_rename_with_acl() function uses sys$get_security() and sys$set_security(), but the code in mp_do_kill_file() uses the obsolete sys$change_acl(), which isn't documented, except in a table under "Obsolete services" in the second volume of the System Services Reference Manual. Clang is complaining that sys$change_acl() takes 9 parameters instead of 7, and since it's an obsolete service, it's easier to clean it up.

There's a comment in the code noting these issues, so I decided I may as well do the cleanup myself, making two new functions to add and remove the ACL from the file, which simplifies the implementation of the rename and delete functions. The original versions also seem to be doing a lot of extra work, finding an ACL in the file, saving a copy, adding the new one, removing it, and then adding back the first found ACL, which doesn't appear to be necessary, since a new ACL was added, so there shouldn't have been anything replaced.

Regards,
Jake Hamby

On 10/21/23 11:07 PM, Jake Hamby (Solid State Jake) wrote:
> On Saturday, October 21, 2023 at 7:35:04 PM UTC-7, Craig A. Berry wrote:
>> On 10/21/23 8:52 PM, Arne Vajhøj wrote:
>>> Only one place to change the name and less risk to mess
>>> up if one need to change from a constant string to a
>>> pointer like argv[1].
>> That does look better, but unfortunately while these suggestions work
>> for C, there is a warning with clang++:
>>
>> $ cxx/pointer_size=64=argv crelnm64.c/noopt/debug
>> DKA0:[CRAIG.test]crelnm64.c;9:18:17: warning: conversion from string
>> literal to 'char * __ptr64' is deprecated
>> [-Wc++11-compat-deprecated-writable
>> -strings]
>> char *tab = "LNM$PROCESS";
>> ^
>> 1 warning generated.
>
> Yes, I ran into that ...

The following seems to work for both cc and clang++:

const char* const tab = "LNM$PROCESS";
struct dsc64$descriptor_s d_tab = {1, DSC$K_DTYPE_T,
DSC$K_CLASS_S, -1, strlen(tab), (char *)tab};

This gadget helps explain what the double consting is doing:

https://cdecl.org

"declare tab as const pointer to const char"

Trying to initialize a value from a string literal has turned into
serious work in recent versions of C++.

On 10/22/2023 12:07 AM, Jake Hamby (Solid State Jake) wrote:
> Changing the type to "unsigned int" makes the warning disappear. Does
> this mean that the default behavior of Clang is to truncate long
> pointers to short ones silently? Or does it somehow know that local
> variables on the stack and global variables are in 32-bit space? In
> that case, what about malloc()'d memory? Something doesn't quite add
> up here.

Local/stack variables are in P1 space, which is accessible
via both 32 bit and 64 bit pointers.

Malloc'ed variables are either in P0 space or P2 space. Where
depends on whether the malloc call end up as _malloc32
or _malloc464. P0 space is accessible via both 32 bit and
64 bit pointers. P2 space is only accessible via 64 bit
pointers - not via 32 bit pointers.

You may want to skim through:
https://www.vajhoej.dk/arne/articles/vms64.html

Arne

On Sunday, October 22, 2023 at 8:56:26 AM UTC-4, Arne Vajhøj wrote:
> On 10/22/2023 12:07 AM, Jake Hamby (Solid State Jake) wrote:
> > Changing the type to "unsigned int" makes the warning disappear. Does
> > this mean that the default behavior of Clang is to truncate long
> > pointers to short ones silently? Or does it somehow know that local
> > variables on the stack and global variables are in 32-bit space? In
> > that case, what about malloc()'d memory? Something doesn't quite add
> > up here.
> Local/stack variables are in P1 space, which is accessible
> via both 32 bit and 64 bit pointers.
>
> Malloc'ed variables are either in P0 space or P2 space. Where
> depends on whether the malloc call end up as _malloc32
> or _malloc464. P0 space is accessible via both 32 bit and
> 64 bit pointers. P2 space is only accessible via 64 bit
> pointers - not via 32 bit pointers.
>
> You may want to skim through:
> https://www.vajhoej.dk/arne/articles/vms64.html
>
> Arne
I'll have one of the team read thru this long thread and pick out issues for our bug list. Thanks for trying this and giving feedback.

John

On 2023-10-22, John Reagan <xyzzy1959@gmail.com> wrote:
> I'll have one of the team read thru this long thread and pick out issues for our bug list. Thanks for trying this and giving feedback.
>

Please consider feeding back my documentation suggestions to the
documentation team. The environmental assumptions driving the
wording of the documentation have changed since that documentation
was written.

Simon.

--
Simon Clubley, clubley@remove_me.eisner.decus.org-Earth.UFP
Walking destinations on a map are further away than they appear.