clisp-devel

Cygwin build fix

[Vibhu M. <vi...@ho...>]

Hi,

I've found and fixed a problem in lispbibl.d. I've attached an 
indicative diff of the C file.

System: Windows 10, a fresh Cygwin, and the latest GitLab clisp.

I followed the instructions in INSTALL.windows ("3. Binaries for the 
Cygwin environment"), for 64-bit binaries. Besides the prescribed 
configure options, I added --ignore-absence-of-libsigsegv.

Make fails to produce spvw.o because lispbibl.c does this near line 1900:
#define CR 13
and then #includes a file (unix.c) that ultimate #includes
/usr/include/w32api/winnt.h
which itself has a legitimate symbol (not macro) named CR that gets 
replaced by 13 resulting in a syntax error.

My fix is to move the whole #define block a few lines down to after the 
#include block. That prevents the #define macros from inadvertently 
capturing symbols from the included files.

----
I'm not an expert C programmer, but this technique of #including a file 
after #defining a macro seems wrong in general. It makes changing the 
included file impossibly hard as its author isn't free to use any new 
symbols without fearing that an includer like clisp will capture them. 
If clisp files followed a rule of never allowing #includes to follow 
#defines, but to only ever precede them, I think that would consistently 
prevent this particular sort of variable capture regardless of the depth 
of an include hierarchy. An alternative prevention would be if include 
file authors only created symbols with some namespace prefix, though 
that would make their code unreadable.


With best wishes,

Vibhu

Re: Cygwin build fix

[Bruno H. <br...@cl...>]

Vibhu Mohindra wrote:
> I've found and fixed a problem in lispbibl.d. I've attached an 
> indicative diff of the C file.
> 
> System: Windows 10, a fresh Cygwin, and the latest GitLab clisp.
> 
> I followed the instructions in INSTALL.windows ("3. Binaries for the 
> Cygwin environment"), for 64-bit binaries. Besides the prescribed 
> configure options, I added --ignore-absence-of-libsigsegv.
> 
> Make fails to produce spvw.o because lispbibl.c does this near line 1900:
> #define CR 13
> and then #includes a file (unix.c) that ultimate #includes
> /usr/include/w32api/winnt.h
> which itself has a legitimate symbol (not macro) named CR that gets 
> replaced by 13 resulting in a syntax error.

Thanks for reporting this.

> My fix is to move the whole #define block a few lines down to after the 
> #include block. That prevents the #define macros from inadvertently 
> capturing symbols from the included files.

I think a less intrusive fix would be this one:

diff --git a/src/unix.d b/src/unix.d
index c600b6cbb..f82b9f157 100644
--- a/src/unix.d
+++ b/src/unix.d
@@ -566,7 +566,10 @@ extern int wait2 (pid_t pid); /* see unixaux.d */
   #define WIN32_LEAN_AND_MEAN
   #pragma push_macro ("Handle")
   #undef Handle
+  #pragma push_macro ("CR")
+  #undef CR
   #include <windows.h>
+  #pragma pop_macro ("CR")
   #pragma pop_macro ("Handle")
   #undef WIN32
   extern long time_t_from_filetime (const FILETIME * ptr);

Can you please try it out?

> ----
> I'm not an expert C programmer, but this technique of #including a file 
> after #defining a macro seems wrong in general. It makes changing the 
> included file impossibly hard as its author isn't free to use any new 
> symbols without fearing that an includer like clisp will capture them. 
> If clisp files followed a rule of never allowing #includes to follow 
> #defines, but to only ever precede them, I think that would consistently 
> prevent this particular sort of variable capture regardless of the depth 
> of an include hierarchy.

Such a rule is too rigid, and gets in the way of building larger software.
Most importantly, for maintainable software, one should follow the principle
that pieces of code that are semantically related should be close together.
Putting the #includes in one place, the macros in a second place, the function
declarations in a third place, and the function definitions in a fourth place
tends to go against maintainability. (Early PASCAL was criticized for this:
constants had to be defined first, then the types, then the functions, and
finally the main program.)

It's better to not have this rule, and deal with the occasional (rare)
conflicts on a case-by-case basis.

> An alternative prevention would be if include 
> file authors only created symbols with some namespace prefix, though 
> that would make their code unreadable.

Yes. Unfortunatelty, namespaces are a feature of newer programming languages,
like C++ and Common Lisp itself, but not of C.

Bruno

Re: Cygwin build fix

[Vibhu M. <vi...@ho...>]

Hi Bruno,

On 25/09/2022 04:41, Bruno Haible wrote:
> Vibhu Mohindra wrote:
>> My fix is to move the whole #define block a few lines down to after the
>> #include block. That prevents the #define macros from inadvertently
>> capturing symbols from the included files.
> 
> I think a less intrusive fix would be this one:
> 
> diff --git a/src/unix.d b/src/unix.d
> index c600b6cbb..f82b9f157 100644
> --- a/src/unix.d
> +++ b/src/unix.d
> @@ -566,7 +566,10 @@ extern int wait2 (pid_t pid); /* see unixaux.d */
>     #define WIN32_LEAN_AND_MEAN
>     #pragma push_macro ("Handle")
>     #undef Handle
> +  #pragma push_macro ("CR")
> +  #undef CR
>     #include <windows.h>
> +  #pragma pop_macro ("CR")
>     #pragma pop_macro ("Handle")
>     #undef WIN32
>     extern long time_t_from_filetime (const FILETIME * ptr);
> 
> Can you please try it out?

Tried your fix instead of mine. Worked perfectly.

[Separate matter:
As before "make check" reports exactly one failure due to trying to open 
/etc/passwd, which Cygwin doesn't have. I can't think of a file that's 
definitely available on every platform, maybe /dev/zero or something or 
even ./unix.c. This is tests/exceptsit.erg:
Form: (PROGN (OPEN "/etc/passwd" :DIRECTION :INPUT :IF-EXISTS :ERROR) (/ 0))
CORRECT: DIVISION-BY-ZERO
CLISP  : #<SYSTEM::SIMPLE-FILE-ERROR #x00000008000A1D59>
]

>> ----
>> I'm not an expert C programmer, but this technique of #including a file
>> after #defining a macro seems wrong in general. It makes changing the
>> included file impossibly hard as its author isn't free to use any new
>> symbols without fearing that an includer like clisp will capture them.
>> If clisp files followed a rule of never allowing #includes to follow
>> #defines, but to only ever precede them, I think that would consistently
>> prevent this particular sort of variable capture regardless of the depth
>> of an include hierarchy.
> 
> Such a rule is too rigid, and gets in the way of building larger software.
> Most importantly, for maintainable software, one should follow the principle
> that pieces of code that are semantically related should be close together.
> Putting the #includes in one place, the macros in a second place, the function
> declarations in a third place, and the function definitions in a fourth place
> tends to go against maintainability. (Early PASCAL was criticized for this:
> constants had to be defined first, then the types, then the functions, and
> finally the main program.)
> 
> It's better to not have this rule, and deal with the occasional (rare)
> conflicts on a case-by-case basis.

I see. So this isn't a new kind of problem. Everyone's obviously seen it 
before, and invented this macro pushing/popping. Cool stuff. Thanks for 
explaining.

>> An alternative prevention would be if include
>> file authors only created symbols with some namespace prefix, though
>> that would make their code unreadable.
> 
> Yes. Unfortunatelty, namespaces are a feature of newer programming languages,
> like C++ and Common Lisp itself, but not of C.

Oh right, I just meant namespaces the hard way: E.g. winnt.h would use 
_WINNT_CR rather than bare CR. But in any case, it would mean the whole 
world would have to agree on the convention before it could be useful, 
and it would look ugly anyway. Not as practical as the PASCAL or 
push/pop techniques.


Thanks a lot,

Vibhu

Re: Cygwin build fix

[Bruno H. <br...@cl...>]

Vibhu Mohindra wrote:
> > Can you please try it out?
> 
> Tried your fix instead of mine. Worked perfectly.

Thank you for the quick return. I have now pushed the fix.

> [Separate matter:
> As before "make check" reports exactly one failure due to trying to open 
> /etc/passwd, which Cygwin doesn't have. I can't think of a file that's 
> definitely available on every platform, maybe /dev/zero or something or 
> even ./unix.c. This is tests/exceptsit.erg:
> Form: (PROGN (OPEN "/etc/passwd" :DIRECTION :INPUT :IF-EXISTS :ERROR) (/ 0))
> CORRECT: DIVISION-BY-ZERO
> CLISP  : #<SYSTEM::SIMPLE-FILE-ERROR #x00000008000A1D59>
> ]

Since you have checked that this file does not exist on your system, you can
obviously ignore the error. You're right that this file usually does not
exist any more on Cygwin: <https://stackoverflow.com/questions/28573763>.

Bruno