Summary: | [EFL] Build warning: Fix warn_unused_result warnings | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Byungwoo Lee <bw80.lee> | ||||||
Component: | WebKit EFL | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | gyuyoung.kim, lucas.de.marchi, rakuco, s.choi, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Byungwoo Lee
2012-02-21 22:04:56 PST
Changed title to '[EFL] Build warning: Fix warn_unused_result warnings'. And re-scoping this bug only for warn_unused_result warning. ** On release build ($ ./Tools/Scripts/build-webkit --efl) ... [100%] Building C object Tools/EWebLauncher/CMakeFiles/bin/EWebLauncher.dir/main.c.o /WebKit/Tools/DumpRenderTree/efl/ImageDiff.cpp: In function void printImage(Evas_Object*): /WebKit/Tools/DumpRenderTree/efl/ImageDiff.cpp:213: warning: ignoring return value of ssize_t write(int, const void*, size_t), declared with attribute warn_unused_result ... ** On build for DRT test ($ ./Tools/Scripts/build-webkit --efl --touch-events --touch-icon-loading \ --blob --device-orientation --fullscreen-api \ --geolocation --notifications --orientation-events \ --video --workers --shared-workers --request-animation-frame \ --page-visibility-api --cmakearg="-DSHARED_CORE=ON") ... [ 96%] Building CXX object Source/WebCore/CMakeFiles/webcore_efl.dir/platform/efl/SharedBufferEfl.cpp.o /WebKit/Source/WebCore/platform/efl/SharedBufferEfl.cpp: In static member function static WTF::PassRefPtr<WebCore::SharedBuffer> WebCore::SharedBuffer::createWithContentsOfFile(const WTF::String&): /WebKit/Source/WebCore/platform/efl/SharedBufferEfl.cpp:64: warning: ignoring return value of size_t fread(void*, size_t, size_t, FILE*), declared with attribute warn_unused_result ... Created attachment 130263 [details]
Patch
Comment on attachment 130263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130263&action=review > Source/WebCore/platform/efl/SharedBufferEfl.cpp:71 > - fread(result->m_buffer.data(), 1, fileStat.st_size, file); > + if (static_cast<unsigned>(fileStat.st_size) != fread(result->m_buffer.data(), 1, fileStat.st_size, file)) { > + fclose(file); > + return 0; > + } > + > fclose(file); > > return result.release(); How about const size_t freadResult = fread(...); fclose(file); const bool readSuccessful = (freadResult == static_cast<size_t>(fileStat.st_size)); return readSucessful ? result.relase() : 0; > Tools/DumpRenderTree/efl/ImageDiff.cpp:218 > + } while ((bytesWritten += count) < bytesRead); Personally, I find (bytesWritten += count) here weird to read (no pun intended :-). How about moving the increase operation inside the do-while loop? Thanks for your comment. > How about > > const size_t freadResult = fread(...); > fclose(file); > > const bool readSuccessful = (freadResult == static_cast<size_t>(fileStat.st_size)); > > return readSucessful ? result.relase() : 0; This looks better for me also, and I think 'readSuccessful' is redundant. How about the below? - fread(result->m_buffer.data(), 1, fileStat.st_size, file); + const size_t bytesRead = fread(result->m_buffer.data(), 1, fileStat.st_size, file); fclose(file); - return result.release(); + return bytesRead == static_cast<unsigned>(fileStat.st_size) ? result.release() : 0; > > Tools/DumpRenderTree/efl/ImageDiff.cpp:218 > > + } while ((bytesWritten += count) < bytesRead); > > Personally, I find (bytesWritten += count) here weird to read (no pun intended :-). How about moving the increase operation inside the do-while loop? Ok~ I'll fix as below + bytesWritten += count; + } while (bytesWritten < bytesRead); (In reply to comment #4) > How about the below? > > - fread(result->m_buffer.data(), 1, fileStat.st_size, file); > + const size_t bytesRead = fread(result->m_buffer.data(), 1, fileStat.st_size, file); > fclose(file); > > - return result.release(); > + return bytesRead == static_cast<unsigned>(fileStat.st_size) ? result.release() : 0; That's OK too. Created attachment 130358 [details]
Patch
Comment on attachment 130358 [details]
Patch
r=me.
Comment on attachment 130358 [details] Patch Clearing flags on attachment: 130358 Committed r110035: <http://trac.webkit.org/changeset/110035> All reviewed patches have been landed. Closing bug. |