Some reports on VncViewer

Developers may discuss here.

Some reports on VncViewer

Postby zaksoft » 2013-12-08 12:19

Some reports:

A) ClientConnectionCursor.cpp line 52

if (pfburh->r.x<0)return;
if (pfburh->r.x<0)return;

should be

if (pfburh->r.x<0)return;
if (pfburh->r.y<0)return;


B) FullScreenTitleBar.cpp Lines 411 and 428

ZeroMemory(Text,sizeof(LPTSTR));
should be
ZeroMemory(Text,sizeof(MAX_PATH));

C) VncViewerApp32.cpp Lines 79, 93 and 107
memcpy((char*)&pcc->m_opts,(char*)&m_options,sizeof(m_options));

m_options is an instance of VNCOptions that has virtual functions, memcpy can damage VTable. I do not see any derived class, so virtual can be removed from destrcuctor. Inserting a copy constructor is always the better solution-

Hope this help.
zaksoft
 
Posts: 2
Joined: 2013-12-08 10:19

Re: Some reports on VncViewer

Postby Rudi De Vos » 2013-12-08 21:33

A) error, strange that new code analyser of 2013 didn't found it.
B) don't see the error
TCHAR Text[MAX_PATH];
ZeroMemory(Text,sizeof(Text));
sizeof(int)-> 4
sizeof(txt)==sizeof(TCHAR)*MAX_PATH
C) checking
Rudi De Vos
Admin & Developer
Admin & Developer
 
Posts: 5485
Joined: 2004-04-23 10:21

Re: Some reports on VncViewer

Postby zaksoft » 2013-12-10 16:31

B) FullScreenTitleBar.cpp Lines 411 and 428

ZeroMemory(Text,sizeof(LPTSTR));
should be (sorry, sizeof below is a copy and paste error)
ZeroMemory(Text,sizeof(TCHAR) * MAX_PATH);

In original code you force memset to 0 for 4 bytes, you can use as you wrote
ZeroMemory(Text,sizeof(Text));
not
ZeroMemory(Text,sizeof(LPTSTR));
zaksoft
 
Posts: 2
Joined: 2013-12-08 10:19

Re: Some reports on VncViewer

Postby Rudi De Vos » 2013-12-10 18:20

Looks like you are not using the latest src.
ZeroMemory(Text,sizeof(Text)); is how it's in the svn
http://sourceforge.net/p/ultravnc/code/HEAD/tarball
Rudi De Vos
Admin & Developer
Admin & Developer
 
Posts: 5485
Joined: 2004-04-23 10:21


Return to Developer discussions (mainly user-mode)

Who is online

Users browsing this forum: No registered users and 2 guests