[PATCH FOR REVIEW]: Fix warning in src/share/vm/adlc/archDesc.cpp

[David Schlosnagle]

Andrew,

It seems like the warning and assert actually pointed out an existing
bug -- the second and third arguments to fwrite seem to be swapped.
The second arg should be the size (1 for char) and the third argument
should is the number of items to write.

The following should work:
//---------------------------addSUNcopyright-------------------------------
 // output SUN copyright info
 void ArchDesc::addSunCopyright(char* legal, int size, FILE *fp) {
-  fwrite(legal, size, 1, fp);
+  size_t count = fwrite(legal, 1, size, fp);
+  assert(count == size, "copyright info truncated");
   fprintf(fp,"\n");
   fprintf(fp,"// Machine Generated File.  Do Not Edit!\n");
   fprintf(fp,"\n");
 }

>From <http://docs.sun.com/app/docs/doc/816-5168/fwrite-3c?a=view>:
Synopsis
    #include <stdio.h>
    size_t fwrite(const void *ptr, size_t size, size_t nitems, FILE *stream);

Description
    The fwrite() function writes, from the array pointed to by ptr, up to
    nitems elements whose size is specified by size, to the stream pointed to
    by stream. For each object, size calls are made to the fputc(3C) function,
    taking the values (in order) from an array of unsigned char exactly
    overlaying the object. The file-position indicator for the stream (if
    defined) is advanced by the number of bytes successfully written. If an
    error occurs, the resulting value of the file-position indicator for the
    stream is unspecified.

    The st_ctime and st_mtime fields of the file will be marked for update
    between the successful execution of fwrite() and the next successful
    completion of a call to fflush(3C) or fclose(3C) on the same stream or a
    call to exit(2) or abort(3C).

Return Values
    The fwrite() function returns the number of elements successfully written,
    which might be less than nitems if a write error is encountered. If size or
    nitems is 0, fwrite() returns 0 and the state of the stream remains
    unchanged. Otherwise, if a write error occurs, the error indicator for the
    stream is set and errno is set to indicate the error.

Thanks,
David Schlosnagle

On Mon, Sep 7, 2009 at 6:40 PM, Andrew John Hughes
<gnu_andrew at member.fsf.org> wrote:
>
> 2009/9/6 Andrew John Hughes <gnu_andrew at member.fsf.org>:
> > 2009/9/5 David Holmes - Sun Microsystems <David.Holmes at sun.com>:
> >> Christian Thalinger said the following on 09/05/09 18:59:
> >>>
> >>> Well, looking more closely it's actually a system header attribute that
> >>> brings up that warning:
> >>>
> >>>
> >>> /mnt/builder/icedtea7/openjdk-ecj/hotspot/src/share/vm/adlc/archDesc.cpp:1034:
> >>> error: ignoring return value of 'size_t fwrite(const void*, size_t,
> >>> size_t, FILE*)', declared with attribute warn_unused_result
> >>>
> >>> And it seems there is no GCC switch to turn that warning off (except
> >>> -O0).  Maybe we should check for the written bytes to be the same as
> >>> requested:
> >>>
> >>>  assert(count == size, "copyright info truncated");
> >>
> >> I recall this coming up in the past. gcc manual lists:
> >>
> >> -Wno-unused-result
> >>
> >> but that might be version specific.
> >>
> >> David
> >>
> >
> > It's a very recent addition:
> >
> > http://gcc.gnu.org/ml/gcc-patches/2009-07/msg00474.html
> >
> > which means there is a period (which includes GCC 4.4) where the
> > warning is produced but can't be disabled :(
> >
> > BTW, my current approach was influenced by
> > http://hg.openjdk.java.net/jdk7/jdk7/hotspot/rev/2328d1d3f8cf where
> > the same fix is applied to remove this warning.
> > I have to say I personally prefer twisti's assert fix and can apply
> > this if others are in agreement.
> > --
> > Andrew :-)
> >
> > Free Java Software Engineer
> > Red Hat, Inc. (http://www.redhat.com)
> >
> > Support Free Java!
> > Contribute to GNU Classpath and the OpenJDK
> > http://www.gnu.org/software/classpath
> > http://openjdk.java.net
> >
> > PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
> > Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8
> >
>
> This is interesting; that assertion fails:
>
> g++ -m64 -Xlinker -O1  -m64 -export-dynamic -o
> ../generated/adfiles/adlc ../generated/adfiles/adlparse.o
> ../generated/adfiles/archDesc.o ../generated/adfiles/arena.o
> ../generated/adfiles/dfa.o ../generated/adfiles/dict2.o
> ../generated/adfiles/filebuff.o ../generated/adfiles/forms.o
> ../generated/adfiles/formsopt.o ../generated/adfiles/formssel.o
> ../generated/adfiles/main.o ../generated/adfiles/adlc-opcodes.o
> ../generated/adfiles/output_c.o ../generated/adfiles/output_h.o
> ../generated/adfiles/adlc -DLINUX -D_GNU_SOURCE -DAMD64 -q -T -D_LP64
> ../generated/adfiles/linux_x86_64.ad  \
>  -c../generated/adfiles/mktmp11816/ad_x86_64.cpp
> -h../generated/adfiles/mktmp11816/ad_x86_64.hpp
> -a../generated/adfiles/mktmp11816/dfa_x86_64.cpp
> -v../generated/adfiles/mktmp11816/adGlobals_x86_64.hpp \
>            || { rm -rf ../generated/adfiles/mktmp11816; exit 1; }
> assert fails /mnt/builder/icedtea7/openjdk-ecj/hotspot/src/share/vm/adlc/archDesc.cpp
> 1035: copyright info truncated
> /bin/sh: line 2: 12016 Aborted
> ../generated/adfiles/adlc -DLINUX -D_GNU_SOURCE -DAMD64 -q -T -D_LP64
> ../generated/adfiles/linux_x86_64.ad
> -c../generated/adfiles/mktmp11816/ad_x86_64.cpp
> -h../generated/adfiles/mktmp11816/ad_x86_64.hpp
> -a../generated/adfiles/mktmp11816/dfa_x86_64.cpp
> -v../generated/adfiles/mktmp11816/adGlobals_x86_64.hpp
> make[7]: *** [refresh_adfiles] Error 1
>
> --
> Andrew :-)
>
> Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
>
> Support Free Java!
> Contribute to GNU Classpath and the OpenJDK
> http://www.gnu.org/software/classpath
> http://openjdk.java.net
>
> PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
> Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8