bugGNU Octave - Bugs: bug #55366, Second argument to save_binary is...

 
 

bug #55366: Second argument to save_binary is a non-constant reference

Submitter:  Rik <rik5>
Submitted:  Thu 03 Jan 2019 05:25:24 AM UTC
   
 
Category:  None Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 03 Jan 2019 04:28:47 PM UTC, comment #3: 

OK, I pushed it on default and merged stable to default.

John W. Eaton <jwe>
Group administrator
Thu 03 Jan 2019 04:16:49 PM UTC, comment #2: 

I think its best on the default branch.  The change is simple, but it is widespread.  I also like passing it by value.  A bool is likely to take just 1 byte, whereas passing a reference to a bool is likely to take 8 bytes (size of pointer).

Rik <rik5>
Group administrator
Thu 03 Jan 2019 02:39:26 PM UTC, comment #1: 

I did not look at the detailed history of this function, but it is possible that the reference parameter was originally intended to allow information to be passed back to the caller.  So first I changed it to use "const bool&" to see whether there were any cases in which the argument is modified, but it is not.  Now I see that you said that the issue was identified by the static analysis tool, so it probably found that parameter passed by reference was never modified.  Since this is a POD type, we should just pass it by value.  I'm attaching a changeset.

I was working on the stable branch when I created the changeset, but it if you would prefer to defer the change to the next major release that is also OK with me.


(file #45853)

John W. Eaton <jwe>
Group administrator
Thu 03 Jan 2019 05:25:24 AM UTC, original submission:  

The prototype for the function save_binary has a reference to bool (bool& save_as_floats) as the second function argument.

It is very likely that this should either be a constant reference or the reference should be skipped entirely and it should just be a bool passed by value.

This issue was identified by static analysis of the code.  If this is indeed the case, then a whole bunch of function prototypes need to change starting with ov.h and proceeding in to each octave_value class.

Rik <rik5>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #45853:  save-binary-diffs.txt added by jwe (31KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by rik5 (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-01-03 jwe StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2019-01-03 rik5 StatusNone Confirmed
    2019-01-03 jwe Attached File- Added save-binary-diffs.txt, #45853

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code