7615 libbe: cleanup compile warnings

Review Request #274 — Created Nov. 24, 2016 and submitted

tsoome
illumos-gate
7615
63ff41e...
general
7615 libbe: cleanup compile warnings

clean build. tested on x86 and sparc.

andy_js
  1. Is it really necessary to pass -Wall to the compiler? I thought we did that already via the compiler wrapper.

    1. Yes we did. And at once are suppressing the same warnings, an example compile line for be_utils.c:

      /opt/gcc/4.4.4/bin/gcc -fident -finline -fno-inline-functions -fno-builtin -fno-asm -fdiagnostics-show-option -nodefaultlibs -D__sun -fno-strict-aliasing -fno-unit-at-a-time -fno-optimize-sibling-calls -O2 -m64 -mtune=opteron -Ui386 -U__i386 -Wall -Wextra -Werror -Wno-missing-braces -Wno-sign-compare -Wno-unknown-pragmas -Wno-unused-parameter -Wno-missing-field-initializers -Wno-array-bounds -Wno-unused -Wno-empty-body -Wall -std=gnu99 -fno-inline-small-functions -fno-inline-functions-called-once -fno-ipa-cp -DTEXT_DOMAIN="SUNW_OST_OSLIB" -D_TS_ERRNO -I/code/illumos-gate/proto/root_i386/usr/include -I../common -I/code/illumos-gate/usr/src/cmd/boot/common -I/code/illumos-gate/usr/src/common/ficl -c -o objs/be_utils.o ../common/be_utils.c 
      

      See how we switch on -Wall -Wextra -Werror and then are switching off the warnings... the last -Wall is coming from libbe Makefile.com (and should be removed once the build system is sane about warning management).

    2. I would like to see a comment to that effect then. It looks like you've tackled the pragma problem, so perhaps we can remove -Wno-unknown-pragmas from the default compiler flags?

    3. I can make an experiment, but I think removing it from Makefile.lib (or wherever it is set), will break more libs and I would like to "chew" this issue with smaller chunks - as when headers are getting cleaned, the followup changes will get smaller and more manageable.

      Thats the reason I would rather limit the scope related to libbe this time. And yes, as I wrote to list, the -Wall option is set there on purpose, and yes, it means it should be cleared later.

  2. 
      
tsoome
rm
  1. While I understand why we're cleaning up the #pragma ident's and why you want to remove the control flow related pragmas, don't the control flow related pragma's still have meaning to non-gcc compilers (e.g. Studio)? Rather than removing them, shouldn't we be adding something to sys/ccompile.h and then making sure it either expands to the corresponding gcc attribute or not at all?
    1. This is good point - I actually did try to search for the documentation for it - and failed. Probably should look more. From gcc point of view this pragma is unknown, that much is clear. Do we actually need to care about studio there, especially as the studio documentation does not even seem to refer to this pragma at all (at least to the extent I did search). So what is/was the actual meaning for this pragma in first place - sure we can try to guess from its name and what [v]fork and setjump does, but still..

    2. https://docs.oracle.com/cd/E19205-01/819-5265/bjaby/index.html

    3. Yap, somehow I did manage to miss it. Anyhow, Robert also did hint on it (altho he found the C++ manual:D) And as suggested by richlowe, I did left them as is for this patch - this one has already long list of updated files.

      For Studio specific pragmas we really should use guards IMO, but thats topic for separate issue.

  2. 
      
tsoome
yuripv
  1. Ship It!
  2. 
      
danmcd
  1. Ship It!
  2. 
      
rm
  1. Ship It!
  2. 
      
tsoome
rm
  1. Ship It!
  2. 
      
danmcd
  1. Still ship it!

  2. 
      
yuripv
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...