#14725: suspicious code in file.cpp

13 views
Skip to first unread message

wxTrac

unread,
Oct 4, 2012, 5:11:37 PM10/4/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14725>

#14725: suspicious code in file.cpp
------------------------------------------+---------------------------------
Reporter: ghostvoodooman | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: base | Version: 2.9-svn
Keywords: file wxFile loop for ReadAll | Blockedby:
Patch: 0 | Blocking:
------------------------------------------+---------------------------------
There is suspicious code in ReadAll() function:

http://svn.wxwidgets.org/viewvc/wx/wxWidgets/trunk/src/common/file.cpp?view=markup

RealAll() function:
{{{
for ( ;; )
{
static const unsigned READSIZE = 4096;

ssize_t nread = Read(p, length > READSIZE ? READSIZE : length);
if ( nread == wxInvalidOffset )
return false;

p += nread;
}

*p = 0;

wxString strTmp(buf, conv);
str->swap(strTmp);

return true;
}}}

compiler warns about line 314 saying non-reachable code
{{{
*p = 0;
}}}

the code is not reachable, since there is infinite loop [namely "for(;;)"
construct], and inside of it there is only conditional "return" statement,
but no "break" statement. It is assumed the loop either loops infinitely
or returns "false".


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14725>

wxTrac

unread,
Oct 4, 2012, 6:39:05 PM10/4/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14725#comment:1>

#14725: suspicious code in file.cpp
------------------------------------------+---------------------------------
Reporter: ghostvoodooman | Owner: vadz
Type: defect | Status: accepted
Priority: normal | Milestone:
Component: base | Version: 2.9-svn
Keywords: file wxFile loop for ReadAll | Blockedby:
Patch: 0 | Blocking:
------------------------------------------+---------------------------------
Changes (by vadz):

* owner: => vadz
* status: new => accepted


Comment:

This is worse than suspicious, it's totally wrong :-( I forgot to finish
writing this function before checking it in somehow. Fixing now, thanks
for noticing this.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14725#comment:1>

wxTrac

unread,
Oct 4, 2012, 6:47:46 PM10/4/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14725#comment:2>

#14725: suspicious code in file.cpp
-----------------------------+----------------------------------------------
Reporter: ghostvoodooman | Owner: vadz
Type: defect | Status: closed
Priority: normal | Milestone:
Component: base | Version: 2.9-svn
Resolution: fixed | Keywords: file wxFile loop for ReadAll
Blockedby: | Patch: 0
Blocking: |
-----------------------------+----------------------------------------------
Changes (by VZ):

* status: accepted => closed
* resolution: => fixed


Comment:

(In [72614]) Fix fatal bug in the recently added wxFile::ReadAll().

Make sure we exit the loop when reading the file in chunks in
wxFile::ReadAll() and add a unit test for it to ensure that it's really
correct.

Closes #14725.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14725#comment:2>
Reply all
Reply to author
Forward
0 new messages