Skip to content

Commit 4e94760

Browse files
YaSuenagYaSuenag
committed
8263135: unique_ptr should not be used for types that are not pointers
Reviewed-by: asemenyuk, herrick
1 parent f71b21b commit 4e94760

File tree

4 files changed

+41
-63
lines changed

4 files changed

+41
-63
lines changed

‎src/jdk.jpackage/windows/native/common/MsiDb.cpp

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -48,7 +48,7 @@ void closeDatabaseView(MSIHANDLE hView) {
4848

4949

5050
namespace {
51-
UniqueMSIHANDLE openDatabase(const tstring& msiPath) {
51+
MSIHANDLE openDatabase(const tstring& msiPath) {
5252
MSIHANDLE h = 0;
5353
const UINT status = MsiOpenDatabase(msiPath.c_str(),
5454
MSIDBOPEN_READONLY, &h);
@@ -57,7 +57,7 @@ UniqueMSIHANDLE openDatabase(const tstring& msiPath) {
5757
<< "MsiOpenDatabase(" << msiPath
5858
<< ", MSIDBOPEN_READONLY) failed", status));
5959
}
60-
return UniqueMSIHANDLE(h);
60+
return h;
6161
}
6262

6363
} // namespace
@@ -115,7 +115,7 @@ DatabaseRecord::DatabaseRecord(unsigned fieldCount) {
115115
JP_THROW(msi::Error(tstrings::any() << "MsiCreateRecord("
116116
<< fieldCount << ") failed", ERROR_FUNCTION_FAILED));
117117
}
118-
handle = UniqueMSIHANDLE(h);
118+
handle = h;
119119
}
120120

121121

@@ -139,7 +139,7 @@ DatabaseRecord& DatabaseRecord::tryFetch(DatabaseView& view) {
139139

140140

141141
DatabaseRecord& DatabaseRecord::setString(unsigned idx, const tstring& v) {
142-
const UINT status = MsiRecordSetString(handle.get(), idx, v.c_str());
142+
const UINT status = MsiRecordSetString(handle, idx, v.c_str());
143143
if (status != ERROR_SUCCESS) {
144144
JP_THROW(Error(tstrings::any() << "MsiRecordSetString(" << idx
145145
<< ", " << v << ") failed", status));
@@ -149,7 +149,7 @@ DatabaseRecord& DatabaseRecord::setString(unsigned idx, const tstring& v) {
149149

150150

151151
DatabaseRecord& DatabaseRecord::setInteger(unsigned idx, int v) {
152-
const UINT status = MsiRecordSetInteger(handle.get(), idx, v);
152+
const UINT status = MsiRecordSetInteger(handle, idx, v);
153153
if (status != ERROR_SUCCESS) {
154154
JP_THROW(Error(tstrings::any() << "MsiRecordSetInteger(" << idx
155155
<< ", " << v << ") failed", status));
@@ -160,7 +160,7 @@ DatabaseRecord& DatabaseRecord::setInteger(unsigned idx, int v) {
160160

161161
DatabaseRecord& DatabaseRecord::setStreamFromFile(unsigned idx,
162162
const tstring& v) {
163-
const UINT status = MsiRecordSetStream(handle.get(), idx, v.c_str());
163+
const UINT status = MsiRecordSetStream(handle, idx, v.c_str());
164164
if (status != ERROR_SUCCESS) {
165165
JP_THROW(Error(tstrings::any() << "MsiRecordSetStream(" << idx
166166
<< ", " << v << ") failed", status));
@@ -170,7 +170,7 @@ DatabaseRecord& DatabaseRecord::setStreamFromFile(unsigned idx,
170170

171171

172172
unsigned DatabaseRecord::getFieldCount() const {
173-
const unsigned reply = MsiRecordGetFieldCount(handle.get());
173+
const unsigned reply = MsiRecordGetFieldCount(handle);
174174
if (int(reply) <= 0) {
175175
JP_THROW(Error(std::string("MsiRecordGetFieldCount() failed"),
176176
ERROR_FUNCTION_FAILED));
@@ -180,7 +180,7 @@ unsigned DatabaseRecord::getFieldCount() const {
180180

181181

182182
int DatabaseRecord::getInteger(unsigned idx) const {
183-
int const reply = MsiRecordGetInteger(handle.get(), idx);
183+
int const reply = MsiRecordGetInteger(handle, idx);
184184
if (reply == MSI_NULL_INTEGER) {
185185
JP_THROW(Error(tstrings::any() << "MsiRecordGetInteger(" << idx
186186
<< ") failed", ERROR_FUNCTION_FAILED));
@@ -199,7 +199,7 @@ void DatabaseRecord::saveStreamToFile(unsigned idx,
199199
DWORD bytes;
200200
do {
201201
bytes = ReadStreamBufferBytes;
202-
const UINT status = MsiRecordReadStream(handle.get(), UINT(idx),
202+
const UINT status = MsiRecordReadStream(handle, UINT(idx),
203203
buffer.data(), &bytes);
204204
if (status != ERROR_SUCCESS) {
205205
JP_THROW(Error(std::string("MsiRecordReadStream() failed"),
@@ -216,25 +216,23 @@ DatabaseView::DatabaseView(const Database& db, const tstring& sqlQuery,
216216
MSIHANDLE h = 0;
217217

218218
// Create SQL query.
219-
for (const UINT status = MsiDatabaseOpenView(db.dbHandle.get(),
219+
for (const UINT status = MsiDatabaseOpenView(db.dbHandle,
220220
sqlQuery.c_str(), &h); status != ERROR_SUCCESS; ) {
221221
JP_THROW(Error(tstrings::any() << "MsiDatabaseOpenView("
222222
<< sqlQuery << ") failed", status));
223223
}
224224

225-
UniqueMSIHANDLE tmp(h);
226-
227225
// Run SQL query.
228-
for (const UINT status = MsiViewExecute(h, queryParam.handle.get());
226+
for (const UINT status = MsiViewExecute(h, queryParam.handle);
229227
status != ERROR_SUCCESS; ) {
228+
closeMSIHANDLE(h);
230229
JP_THROW(Error(tstrings::any() << "MsiViewExecute("
231230
<< sqlQuery << ") failed", status));
232231
}
233232

234233
// MsiViewClose should be called only after
235234
// successful MsiViewExecute() call.
236-
handle = UniqueDbView(h);
237-
tmp.release();
235+
handle = h;
238236
}
239237

240238

@@ -253,7 +251,7 @@ DatabaseRecord DatabaseView::tryFetch() {
253251

254252
// Fetch data from executed SQL query.
255253
// Data is stored in a record object.
256-
for (const UINT status = MsiViewFetch(handle.get(), &h);
254+
for (const UINT status = MsiViewFetch(handle, &h);
257255
status != ERROR_SUCCESS; ) {
258256
if (status == ERROR_NO_MORE_ITEMS) {
259257
return DatabaseRecord();
@@ -264,14 +262,14 @@ DatabaseRecord DatabaseView::tryFetch() {
264262
}
265263

266264
DatabaseRecord reply;
267-
reply.handle = UniqueMSIHANDLE(h);
265+
reply.handle = h;
268266
return reply;
269267
}
270268

271269

272270
DatabaseView& DatabaseView::modify(const DatabaseRecord& record,
273271
MSIMODIFY mode) {
274-
const UINT status = MsiViewModify(handle.get(), mode, record.handle.get());
272+
const UINT status = MsiViewModify(handle, mode, record.handle);
275273
if (status != ERROR_SUCCESS) {
276274
JP_THROW(Error(tstrings::any() << "MsiViewModify(mode=" << mode
277275
<< ") failed", status));

‎src/jdk.jpackage/windows/native/common/MsiDb.h

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -34,27 +34,9 @@
3434

3535
class Guid;
3636

37-
/**
38-
* Helpers to interact with MSI through database interface.
39-
*/
40-
4137
namespace msi {
42-
void closeDatabaseView(MSIHANDLE h);
43-
44-
struct MsiDbViewDeleter {
45-
typedef MSIHANDLE pointer;
46-
47-
void operator()(MSIHANDLE h) {
48-
closeDatabaseView(h);
49-
}
50-
};
51-
} // namespace msi
5238

53-
54-
typedef std::unique_ptr<MSIHANDLE, msi::MsiDbViewDeleter> UniqueDbView;
55-
56-
57-
namespace msi {
39+
void closeDatabaseView(MSIHANDLE h);
5840

5941
class CA;
6042
class DatabaseView;
@@ -95,6 +77,12 @@ class Database {
9577
*/
9678
explicit Database(const CA& ca);
9779

80+
~Database() {
81+
if (dbHandle != 0) {
82+
closeMSIHANDLE(dbHandle);
83+
}
84+
}
85+
9886
/**
9987
* Returns value of property with the given name.
10088
* Throws NoMoreItemsError if property with the given name doesn't exist
@@ -116,7 +104,7 @@ class Database {
116104
Database& operator=(const Database&);
117105
private:
118106
const tstring msiPath;
119-
UniqueMSIHANDLE dbHandle;
107+
MSIHANDLE dbHandle;
120108
};
121109

122110
typedef std::unique_ptr<Database> DatabasePtr;
@@ -128,7 +116,8 @@ class DatabaseRecord {
128116
}
129117

130118
DatabaseRecord(const DatabaseRecord& other): handle(MSIHANDLE(0)) {
131-
handle.swap(other.handle);
119+
handle = other.handle;
120+
other.handle = 0;
132121
}
133122

134123
DatabaseRecord& operator=(const DatabaseRecord& other);
@@ -141,6 +130,12 @@ class DatabaseRecord {
141130
fetch(view);
142131
}
143132

133+
~DatabaseRecord() {
134+
if (handle != 0) {
135+
closeMSIHANDLE(handle);
136+
}
137+
}
138+
144139
DatabaseRecord& fetch(DatabaseView& view);
145140

146141
DatabaseRecord& tryFetch(DatabaseView& view);
@@ -160,15 +155,15 @@ class DatabaseRecord {
160155
void saveStreamToFile(unsigned idx, const tstring& path) const;
161156

162157
bool empty() const {
163-
return 0 == handle.get();
158+
return 0 == handle;
164159
}
165160

166161
MSIHANDLE getHandle() const {
167-
return handle.get();
162+
return handle;
168163
}
169164

170165
private:
171-
mutable UniqueMSIHANDLE handle;
166+
mutable MSIHANDLE handle;
172167
};
173168

174169

@@ -186,7 +181,7 @@ class DatabaseView {
186181
private:
187182
tstring sqlQuery;
188183
const Database& db;
189-
UniqueDbView handle;
184+
MSIHANDLE handle;
190185
};
191186

192187
} // namespace msi

‎src/jdk.jpackage/windows/native/common/MsiUtils.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -199,7 +199,7 @@ void closeMSIHANDLE(MSIHANDLE h) {
199199
// However it can't access handy msi::getProperty() from that location.
200200
tstring DatabaseRecord::getString(unsigned idx) const {
201201
return ::msi::getProperty(MsiRecordGetString, "MsiRecordGetString",
202-
handle.get(), UINT(idx));
202+
handle, UINT(idx));
203203
}
204204

205205

‎src/jdk.jpackage/windows/native/common/MsiUtils.h

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -32,7 +32,6 @@
3232
#include <stdexcept>
3333
#include <new>
3434
#include <map>
35-
#include <memory>
3635

3736
#include "ErrorHandling.h"
3837
#include "Toolbox.h"
@@ -45,20 +44,6 @@ namespace msi {
4544

4645
void closeMSIHANDLE(MSIHANDLE h);
4746

48-
struct MsiHandleDeleter {
49-
typedef MSIHANDLE pointer;
50-
51-
void operator()(MSIHANDLE h) {
52-
closeMSIHANDLE(h);
53-
}
54-
};
55-
56-
} // namespace msi
57-
58-
typedef std::unique_ptr<MSIHANDLE, msi::MsiHandleDeleter> UniqueMSIHANDLE;
59-
60-
namespace msi {
61-
6247
tstring getProductInfo(const Guid& productCode, const tstring& prop);
6348

6449
tstring getProductInfo(const std::nothrow_t&, const Guid& productCode,

0 commit comments

Comments
 (0)