Opened 5 years ago

Last modified 4 years ago

#1189 new change

Use the same Windows handle wrapper for extension and installer

Reported by: fhd Assignee:
Priority: P3 Milestone:
Module: Adblock-Plus-for-Internet-Explorer Keywords: installer
Cc: sergz Blocked By:
Blocking: Platform: Internet Explorer
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

Description (last modified by fhd)

Background

ABP for IE generally uses AutoHandle as a Windows handle wrapper, the installer code uses Windows_Handle.

What to change

We should use the same class everywhere. Since Windows_Handle is more extensive and semantically a bit different, porting some of that to AutoHandle could make sense.

Change History (6)

comment:1 Changed 5 years ago by eric@…

typedef handle< HANDLE, Allow_Null, Windows_Generic_Destruction > WindowsHandle
typedef handle< HANDLE, Disallow_Null, Windows_Generic_Destruction > WindowsHandleRef

With these definitions, I believe WindowsHandle is indeed semantically equivalent to AutoHandle.

WindowsHandleRef is the same type as the current (as of this writing) Windows_Handle.

I don't think using the name AutoHandle would be a good idea in the installer code. There are a lot of different kinds of handles required in that code to deal with MSI. Most of them are not Windows OS handles.

comment:2 Changed 5 years ago by fhd

  • Cc sergz added

You're right, AutoHandle is not a good name at all. If it wasn't for other types of handles, "handle" generally means an object that handles ownership to some other resource - like smart pointers. WindowsHandle would be much better.

Now that we're using C++11, I'd actually prefer to turn these things into unique_ptr typedefs where possible. But we'd be missing out on the PHANDLE cast operator in the case of AutoHandle, and various things for the installer handles (e.g. not nullable ones).

Adding Serge since I'm not overly into WinAPI - what do you think?

comment:3 Changed 5 years ago by fhd

  • Keywords installer added

comment:4 Changed 5 years ago by fhd

  • Ready unset

Unsetting ready for now, since this is still under discussion.

comment:5 Changed 5 years ago by fhd

  • Blocked By 1185, 1186 removed
  • Blocking 1184 removed
  • Description modified (diff)
  • Summary changed from [Installer cleanup] Use AutoHandle instead of Windows_Handle to Use the same Windows handle wrapper for extension and installer

Removed this from the installer cleanup meta issue - has a lower priority.

comment:6 Changed 4 years ago by oleksandr

  • Ready set
Note: See TracTickets for help on using tickets.