Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#1283 closed change (fixed)

In PluginClass.cpp wrong usage of memset

Reported by: sergz Assignee: sergz
Priority: P4 Milestone: Adblock-Plus-for-Internet-Explorer-1.3
Module: Adblock-Plus-for-Internet-Explorer Keywords: goodfirstbug
Cc: oleksandr@… Blocked By:
Blocking: Platform: Internet Explorer
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/6237450183639040/
http://codereview.adblockplus.org/5163581322559488/

Description

Background

The code is

wchar_t szClassName[MAX_PATH];
...
memset(szClassName, 0, MAX_PATH);

Apparently it nullifies only the first half of characters. it should be memset(szClassName, 0, MAX_PATH * sizeof(wchar_t));

What to change

  • replace all memset calls by std::array::fill to clear the arrays and change the variable type
  • check all usages of sizeof. Replace sizeof(TYPE_NAME) by sizeof(variable) except for the allocation case. It's safer to use the latter variant.
  • replace zero initialization via ZeroMemory and memset by SomeType var = {}. It is list initialization which falls back on value initialization which initializes the everything to zero if there is no default constructor. We cannot use value initialization right now because it's not supported by c++ compiler from MSVS2012.

Change History (5)

comment:1 Changed 5 years ago by sergz

  • Review URL(s) modified (diff)

comment:2 Changed 5 years ago by oleksandr

  • Cc oleksandr@… added
  • Keywords goodfirstbug added
  • Priority changed from Unknown to P4
  • Ready set

comment:3 Changed 5 years ago by sergz

  • Status changed from new to reviewing

comment:4 Changed 5 years ago by oleksandr

  • Resolution set to fixed
  • Review URL(s) modified (diff)
  • Status changed from reviewing to closed

comment:5 Changed 5 years ago by fhd

  • Milestone set to Adblock-Plus-for-Internet-Explorer-1.3
Note: See TracTickets for help on using tickets.