Backport #2247: Fixing compiler warnings

• Status: Closed
• Assignee: shyouhei (Shyouhei Urabe)

[kot (Міхаіл T.)]

=begin
When debugging a problem, gratuitous compiler warnings often get in the way by obscuring real problems.

This is why projects should try to eliminate them -- so that when a new one pops up, such as when new code is added, or the build is performed on a new platform -- the issue can attract its due attention.

The attached patch fixes warnings, that I observed compiling with ``-Wall -Werror'' on RHEL-5.4/x86_64 system. Please, consider merging these changes in.

The addressed problems are:

• rb_thread_join was not declared (intern.h) with the rest of the rb_thread functions. Private declarations
  existed in some C-files...
• The PUSH_FRAME duly defined the local variable _frame as volatile. However, when the value of _frame was
  assigned to the global ruby_frame, this qualifier was lost. There were as many warnings about this, as
  there are invocations of the PUSH_FRAME macro.
• The little thread_timer() function MUST be declared as returning void*, because it is passed to
  pthread_create(). Compiler warned, that a non-void function lacked the return-statement. I added one...
• regex.c in its error reporting tried to print ptrdiff_t value as int. On 64-bit platforms ptrdiff_t is,
  usually, long int, but that's an impossibly large value for a regular expression's length.
  Explicit casting of the pointer-difference down to int should be fine.
• parse.y defined an unused static function arg_prepend()

Also, when compiling with a recent gcc at optimization level -O2 (or higher), util.c throws a number of warnings about breaking strict-aliasing rules. There is no /easy/ fix for this, but the work-around is to simply compile that one file with -fno-strict-aliasing (or whatever flag is needed for this with non-gcc compilers). A /sample/ patch for this issue is attached separately.

Thanks! Yours,

     -mi

=end

[nobu (Nobuyoshi Nakada)]

=begin

At Thu, 22 Oct 2009 00:59:42 +0900,
Mikhail T. [e-mail] wrote in [ruby-core:26217]:

The attached patch fixes warnings, that I observed compiling
with ``-Wall -Werror'' on RHEL-5.4/x86_64 system. Please,
consider merging these changes in.

Thank you.

The addressed problems are:

• rb_thread_join was not declared (intern.h) with the rest of the rb_thread functions. Private declarations
  existed in some C-files...

Applied in r25431.

• The PUSH_FRAME duly defined the local variable _frame as volatile. However, when the value of _frame was
  assigned to the global ruby_frame, this qualifier was lost. There were as many warnings about this, as
  there are invocations of the PUSH_FRAME macro.

Fixed in r17831.

• The little thread_timer() function MUST be declared as returning void*, because it is passed to
  pthread_create(). Compiler warned, that a non-void function lacked the return-statement. I added one...

Fixed in r23201.

• regex.c in its error reporting tried to print ptrdiff_t value as int. On 64-bit platforms ptrdiff_t is,
  usually, long int, but that's an impossibly large value for a regular expression's length.
  Explicit casting of the pointer-difference down to int should be fine.

Fixed in r20465

• parse.y defined an unused static function arg_prepend()

Applied in r25432.

Also, when compiling with a recent gcc at optimization level
-O2 (or higher), util.c throws a number of warnings about
breaking strict-aliasing rules. There is no /easy/ fix for
this, but the work-around is to simply compile that one file
with -fno-strict-aliasing (or whatever flag is needed for
this with non-gcc compilers). A /sample/ patch for this issue
is attached separately.

Fixed in r23353.

--
Nobu Nakada

=end

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Assigned
• Assignee set to shyouhei (Shyouhei Urabe)

=begin

=end

[kot (Міхаіл T.)]

• File patch-ruby-openssl patch-ruby-openssl added

=begin
The attached new patch almost fixes the OpenSSL-extension. The bulk of it is casts from char* to unsigned char*
with some const-ification thrown in.

This should help concentrate on fixing /real/ problems, such as:

gcc44 -I. -I../.. -I/home/mteterin/ruby-1.8.7-p174 -I/home/mteterin/ruby-1.8.7-p174/ext/openssl -DRUBY_EXTCONF_H="extconf.h" -fPIC -O2 -Wall -Wno-unused -Werror -pipe -march=nocona -c /home/mteterin/ruby-1.8.7-p174/ext/openssl/ossl_pkey_dh.c
cc1: warnings being treated as errors
ext/openssl/ossl_pkey_dh.c: In function 'ossl_dh_initialize':
ext/openssl/ossl_pkey_dh.c:173: error: function called through a non-compatible type
ext/openssl/ossl_pkey_dh.c:173: note: if this code is reached, the program will abort
ext/openssl/ossl_pkey_dh.c: In function 'ossl_dh_to_public_key':
ext/openssl/ossl_pkey_dh.c:333: error: function called through a non-compatible type
ext/openssl/ossl_pkey_dh.c:333: note: if this code is reached, the program will abort
make: *** [ossl_pkey_dh.o] Помилка 1

The above problems -- triggered in several files -- seem to be with OpenSSL, rather than Ruby or its openssl-extension. I'm unsure, if any work-around is possible. These are NOT fixed by the attached patch.
=end

[nobu (Nobuyoshi Nakada)]

=begin
backport r18168?
=end

[kot (Міхаіл T.)]

=begin

backport r18168?

Sorry, I'm unsure, what you mean :( If you suggest, I build a snapshot of the repository -- sorry, I can't do that. We only deal with /released/ versions here -- patched, if need be... Maybe, it is time for another patch-release for the 1.8.7pX branch?

Thanks!
=end

[kot (Міхаіл T.)]

=begin
Also, each occurrence of BIO_reset() triggers a warning like:

ext/openssl/ossl_pkey_dh.c: In function 'ossl_dh_initialize':
ext/openssl/ossl_pkey_dh.c:172: error: value computed is not used

The following command (on Linux -- requires pcre in grep) silences all of them at once:

find ext/openssl -type f -name '*.c' | xargs grep -P -l '\sBIO_reset' | xargs perl -p -i.iorig -e 's/(\s)BIO_reset/\1(void)BIO_rest/'

It changes BIO_reset to (void)BIO_reset...
=end

[shyouhei (Shyouhei Urabe)]

• Status changed from Assigned to Closed