Re: CVS-060720 compilation failure on Solaris 10, patch

[Claes Wallin]

Ludovic Courtès wrote:
> I won't be able to test your patch so here are just a few remarks.

Very helpful, thank you. Correcting my submission according to them.

>> - Compiles and passes standalone tests now. Gets stack overflows on
>>   elisp.test and when trying to start 'guile'.
>
> Did you compile Guile with optimizations?  When using GCC, compiling
> with `-O0' quickly yields stack overflows (to that point that one
> doesn't even get to the REPL).

Ah, of course. Testing now.

>> - The sethostname and PTHREAD_ONCE_INIT conditions should be tested
>>   for instead of just slapping a __sun__ on the workarounds. Please
>>   help.
> 
> Sorry, I don't understand this.

I simply mean that "#ifdef __sun__" is a pretty blunt instrument. There
may exist sun platforms without these problems, and there may exist
non-sun platforms with these problems. So someone well-versed in
autoconf should write tests for a missing sethostname declaration and a
faulty PTHREAD_ONCE_INIT declaration, respectively.

>> - Does any __GNUC__ platform _not_ want alloca.h?
> 
> I don't known.  We should normally not have to touch the `#ifdef's 
> around `alloca ()' since they are the documented way to deal with 
> `alloca ()' [0].  OTOH, the `#ifdef's changed a bit in the Autoconf
> manual (see [0]), so perhaps we should update them accordingly. Would
> this solve your problem?

Yes! Thank you, updated the section to latest autoconf standards.

> Also, when submitting a patch, please follow the GNU coding standard
> for ChangeLog entries and code (see below).

Yes, updating to conform.

>> diff -ur ../guile-core/test-suite/standalone/test-list.c
>>  ...
>>  static void
>> -test_scm_list (void)
>> +test_scm_list ()
>> 
> Is there something wrong with the original form?

scm_boot_guile takes a (void*)() parameter, which test_scm_list needs
to conform to. Documenting this.

>> +static void f() {
>> +  test_long_long ();
>> +  test_ulong_long ();
>> +}
>> +
> 
> This should really be something like:
> 
> static void
> startup ()
> {
>   ...
> }

Ah, yes. Updating.

>>  int
>>  main (int argc, char *argv[])
>>  {
>> -  scm_init_guile();
>> -  test_long_long ();
>> -  test_ulong_long ();
>> +  scm_boot_guile(argc, argv, f, NULL);
> 
> `scm_init_guile ()' is documented and perfectly legal in 1.8.0.  Is
>  there any reason why you removed it from test cases?

Yes, this enables the tests on platforms which not yet have a
scm_init_guile() implementation, like Solaris 10 and Mac OS X.
I have one implementation for Mac OS X coming up as soon as I've fixed
this patch, but haven't found any function similar to thread_get_attr_np
or pthread_get_stackaddress_np on Solaris 10.

> 
> Thanks, Ludovic.

Updated patch coming up. Thanks again for your guidance.

   /c

signature.asc
Description: OpenPGP digital signature