Skip to content
This repository has been archived by the owner on Aug 27, 2022. It is now read-only.

Commit

Permalink
8263135: unique_ptr should not be used for types that are not pointers
Browse files Browse the repository at this point in the history 
Reviewed-by: asemenyuk, herrick
  • Loading branch information
@YaSuenag
YaSuenag committed Mar 9, 2021
1 parent f71b21b commit 4e94760
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 63 deletions.
36 changes: 17 additions & 19 deletions src/jdk.jpackage/windows/native/common/MsiDb.cpp
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -48,7 +48,7 @@ void closeDatabaseView(MSIHANDLE hView) {


namespace {
UniqueMSIHANDLE openDatabase(const tstring& msiPath) {
MSIHANDLE openDatabase(const tstring& msiPath) {
MSIHANDLE h = 0;
const UINT status = MsiOpenDatabase(msiPath.c_str(),
MSIDBOPEN_READONLY, &h);
Expand All @@ -57,7 +57,7 @@ UniqueMSIHANDLE openDatabase(const tstring& msiPath) {
<< "MsiOpenDatabase(" << msiPath
<< ", MSIDBOPEN_READONLY) failed", status));
}
return UniqueMSIHANDLE(h);
return h;
}

} // namespace
Expand Down Expand Up @@ -115,7 +115,7 @@ DatabaseRecord::DatabaseRecord(unsigned fieldCount) {
JP_THROW(msi::Error(tstrings::any() << "MsiCreateRecord("
<< fieldCount << ") failed", ERROR_FUNCTION_FAILED));
}
handle = UniqueMSIHANDLE(h);
handle = h;
}


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


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


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

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


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


int DatabaseRecord::getInteger(unsigned idx) const {
int const reply = MsiRecordGetInteger(handle.get(), idx);
int const reply = MsiRecordGetInteger(handle, idx);
if (reply == MSI_NULL_INTEGER) {
JP_THROW(Error(tstrings::any() << "MsiRecordGetInteger(" << idx
<< ") failed", ERROR_FUNCTION_FAILED));
Expand All @@ -199,7 +199,7 @@ void DatabaseRecord::saveStreamToFile(unsigned idx,
DWORD bytes;
do {
bytes = ReadStreamBufferBytes;
const UINT status = MsiRecordReadStream(handle.get(), UINT(idx),
const UINT status = MsiRecordReadStream(handle, UINT(idx),
buffer.data(), &bytes);
if (status != ERROR_SUCCESS) {
JP_THROW(Error(std::string("MsiRecordReadStream() failed"),
Expand All @@ -216,25 +216,23 @@ DatabaseView::DatabaseView(const Database& db, const tstring& sqlQuery,
MSIHANDLE h = 0;

// Create SQL query.
for (const UINT status = MsiDatabaseOpenView(db.dbHandle.get(),
for (const UINT status = MsiDatabaseOpenView(db.dbHandle,
sqlQuery.c_str(), &h); status != ERROR_SUCCESS; ) {
JP_THROW(Error(tstrings::any() << "MsiDatabaseOpenView("
<< sqlQuery << ") failed", status));
}

UniqueMSIHANDLE tmp(h);

// Run SQL query.
for (const UINT status = MsiViewExecute(h, queryParam.handle.get());
for (const UINT status = MsiViewExecute(h, queryParam.handle);
status != ERROR_SUCCESS; ) {
closeMSIHANDLE(h);
JP_THROW(Error(tstrings::any() << "MsiViewExecute("
<< sqlQuery << ") failed", status));
}

// MsiViewClose should be called only after
// successful MsiViewExecute() call.
handle = UniqueDbView(h);
tmp.release();
handle = h;
}


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

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

DatabaseRecord reply;
reply.handle = UniqueMSIHANDLE(h);
reply.handle = h;
return reply;
}


DatabaseView& DatabaseView::modify(const DatabaseRecord& record,
MSIMODIFY mode) {
const UINT status = MsiViewModify(handle.get(), mode, record.handle.get());
const UINT status = MsiViewModify(handle, mode, record.handle);
if (status != ERROR_SUCCESS) {
JP_THROW(Error(tstrings::any() << "MsiViewModify(mode=" << mode
<< ") failed", status));
Expand Down
47 changes: 21 additions & 26 deletions src/jdk.jpackage/windows/native/common/MsiDb.h
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -34,27 +34,9 @@

class Guid;

/**
* Helpers to interact with MSI through database interface.
*/

namespace msi {
void closeDatabaseView(MSIHANDLE h);

struct MsiDbViewDeleter {
typedef MSIHANDLE pointer;

void operator()(MSIHANDLE h) {
closeDatabaseView(h);
}
};
} // namespace msi


typedef std::unique_ptr<MSIHANDLE, msi::MsiDbViewDeleter> UniqueDbView;


namespace msi {
void closeDatabaseView(MSIHANDLE h);

class CA;
class DatabaseView;
Expand Down Expand Up @@ -95,6 +77,12 @@ class Database {
*/
explicit Database(const CA& ca);

~Database() {
if (dbHandle != 0) {
closeMSIHANDLE(dbHandle);
}
}

/**
* Returns value of property with the given name.
* Throws NoMoreItemsError if property with the given name doesn't exist
Expand All @@ -116,7 +104,7 @@ class Database {
Database& operator=(const Database&);
private:
const tstring msiPath;
UniqueMSIHANDLE dbHandle;
MSIHANDLE dbHandle;
};

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

DatabaseRecord(const DatabaseRecord& other): handle(MSIHANDLE(0)) {
handle.swap(other.handle);
handle = other.handle;
other.handle = 0;
}

DatabaseRecord& operator=(const DatabaseRecord& other);
Expand All @@ -141,6 +130,12 @@ class DatabaseRecord {
fetch(view);
}

~DatabaseRecord() {
if (handle != 0) {
closeMSIHANDLE(handle);
}
}

DatabaseRecord& fetch(DatabaseView& view);

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

bool empty() const {
return 0 == handle.get();
return 0 == handle;
}

MSIHANDLE getHandle() const {
return handle.get();
return handle;
}

private:
mutable UniqueMSIHANDLE handle;
mutable MSIHANDLE handle;
};


Expand All @@ -186,7 +181,7 @@ class DatabaseView {
private:
tstring sqlQuery;
const Database& db;
UniqueDbView handle;
MSIHANDLE handle;
};

} // namespace msi
Expand Down
4 changes: 2 additions & 2 deletions src/jdk.jpackage/windows/native/common/MsiUtils.cpp
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -199,7 +199,7 @@ void closeMSIHANDLE(MSIHANDLE h) {
// However it can't access handy msi::getProperty() from that location.
tstring DatabaseRecord::getString(unsigned idx) const {
return ::msi::getProperty(MsiRecordGetString, "MsiRecordGetString",
handle.get(), UINT(idx));
handle, UINT(idx));
}


Expand Down
17 changes: 1 addition & 16 deletions src/jdk.jpackage/windows/native/common/MsiUtils.h
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -32,7 +32,6 @@
#include <stdexcept>
#include <new>
#include <map>
#include <memory>

#include "ErrorHandling.h"
#include "Toolbox.h"
Expand All @@ -45,20 +44,6 @@ namespace msi {

void closeMSIHANDLE(MSIHANDLE h);

struct MsiHandleDeleter {
typedef MSIHANDLE pointer;

void operator()(MSIHANDLE h) {
closeMSIHANDLE(h);
}
};

} // namespace msi

typedef std::unique_ptr<MSIHANDLE, msi::MsiHandleDeleter> UniqueMSIHANDLE;

namespace msi {

tstring getProductInfo(const Guid& productCode, const tstring& prop);

tstring getProductInfo(const std::nothrow_t&, const Guid& productCode,
Expand Down

0 comments on commit 4e94760

Please sign in to comment.