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

Mon Sep 7 17:32:41 PDT 2009

David,

David Schlosnagle said the following on 09/08/09 09:53:
> 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.

I don't see how it makes a difference: size * 1 == 1 * size

All fwrite will do, from what I've seen, is multiply the two to obtain 
the number of bytes to write.

It may be that Andrew encountered this bug: [BZ #5998]

http://sourceware.org/git/?p=glibc.git;a=commit;h=a7925a24fe104a2ab54fb8a6bdec1e5cf80a8db7

or maybe there really was an error writing the data out.

Cheers,
David Holmes

> 
> 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