Paths

Table of Contentst

Differential D15189

Fix C++ .stl importer unused function result warning
ClosedPublic
Actions

Authored by Iyad Ahmed (iyadahmed2001) on Jun 14 2022, 10:50 AM.

Tags

Subscribers

Sonny Campbell (SonnyCampbell_Unity)

Details

Reviewers

Sybren A. Stüvel (sybren)

Commits

rB5c726dd4ef09: Fix C++ STL importer unused function result warning

Summary

Fix unused function result warnings on Linux for usages of fread in the C++ .stl importer

Diff Detail

Repository: rB Blender

Event Timeline

Iyad Ahmed (iyadahmed2001) requested review of this revision.Jun 14 2022, 10:50 AM

Iyad Ahmed (iyadahmed2001) created this revision.

Iyad Ahmed (iyadahmed2001) mentioned this in D15185: Use BKE_report in the C++ .stl importer.

Sybren A. Stüvel (sybren) requested changes to this revision.Jun 14 2022, 10:56 AM

Sybren A. Stüvel (sybren) added inline comments.

source/blender/io/stl/importer/stl_import.cc
60	I don't quite understand this logic. If this `fread` fails, why is the file considered to be ASCII?
source/blender/io/stl/importer/stl_import_binary_reader.cc
37	This should also print the actual error from the OS. Now there is no way for the reader of the message to understand what's wrong or how to fix it. This can be done, for example, with `perror()`

This revision now requires changes to proceed.Jun 14 2022, 10:56 AM

Sonny Campbell (SonnyCampbell_Unity) added a subscriber: Sonny Campbell (SonnyCampbell_Unity).Jun 14 2022, 12:29 PM

Sonny Campbell (SonnyCampbell_Unity) added inline comments.

source/blender/io/stl/importer/stl_import.cc
62	Could also output the error here if the fread fails

Sybren A. Stüvel (sybren) added inline comments.Jun 14 2022, 12:33 PM

source/blender/io/stl/importer/stl_import.cc
62	That would be good, yes. And probably also abort the import, as I don't think such errors should go unnoticed.

Iyad Ahmed (iyadahmed2001) updated this revision to Diff 52512.Jun 14 2022, 8:02 PM

Iyad Ahmed (iyadahmed2001) marked 4 inline comments as done.

Ping

Sybren A. Stüvel (sybren) requested changes to this revision.Jun 23 2022, 10:55 AM

Sybren A. Stüvel (sybren) added inline comments.

source/blender/io/stl/importer/stl_import.cc
59–64	The same question still applies: why is it okay to silently ignore the error in certain cases?
59–64	This can be refactored into a `stl_import_report_error(file)` function, so that the logic isn't duplicated below.

This revision now requires changes to proceed.Jun 23 2022, 10:55 AM

Ok will make these changes asap, just saw

Iyad Ahmed (iyadahmed2001) updated this revision to Diff 52958.Jun 28 2022, 12:14 AM

Iyad Ahmed (iyadahmed2001) updated this revision to Diff 52959.

Iyad Ahmed (iyadahmed2001) marked 2 inline comments as done.Jun 28 2022, 12:17 AM

👍

This revision is now accepted and ready to land.Jun 30 2022, 10:13 AM

Closed by commit rB5c726dd4ef09: Fix C++ STL importer unused function result warning (authored by Iyad Ahmed (iyadahmed2001), committed by Sybren A. Stüvel (sybren)). · Explain WhyJun 30 2022, 10:14 AM

This revision was automatically updated to reflect the committed changes.

Sybren A. Stüvel (sybren) added a commit: rB5c726dd4ef09: Fix C++ STL importer unused function result warning.

Revision Contents
Changeset List

Path

Size

source/

blender/

io/

stl/

importer/

6 lines

stl_import_binary_reader.cc

4 lines

Diff 52489

source/blender/io/stl/importer/stl_import.cc

Loading...

source/blender/io/stl/importer/stl_import_binary_reader.cc

Loading...