[haiku-commits] haiku: hrev53332 - in src: kits/package add-ons/kernel/bus_managers/acpi tools/locale system/kernel kits/package/hpkg

  • From: waddlesplash <waddlesplash@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 2 Aug 2019 21:17:52 -0400 (EDT)

hrev53332 adds 4 changesets to branch 'master'
old head: 2846db2e997b3bcd4e8e22fce7e5ac55ce8baa18
new head: 677e901f80dcaf6f835603825029f8bc5fbd12f7
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=677e901f80dc+%5E2846db2e997b

----------------------------------------------------------------------------

a4a538db9fa6: ACPI: Avoid discarding a potential error value.
  
  This is part of a new diagnostic I've managed to wire up
  that finds all instances of "status_t" function return
  values being discarded using attribute trickery and some
  creative Clang pragmas. As you might guess, the resultant
  errors list is absolutely gigantic, and most of them are
  ones where the failure condition will never (that we care
  about) be hit.
  
  The ones in this commit likely are no-op changes, but
  they were low-hanging fruit spotted while reviewing
  the larger list. The next commit will bring more
  substantial changes.

dfbf1c8a0ac5: Package Kit: Avoid discarding potential error values.
  
  Spotted by Clang and the [[nodiscard]] patch.

afb345915dd5: linkcatkeys: Avoid discarding potential error values.
  
  Spotted by Clang and the [[nodiscard]] patch.

677e901f80dc: kernel/module: Utilize RecursiveLocker.

                              [ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]

----------------------------------------------------------------------------

10 files changed, 48 insertions(+), 52 deletions(-)
.../kernel/bus_managers/acpi/ACPICAHaiku.cpp     |  7 ++-
.../bus_managers/acpi/EmbeddedController.h       |  6 +--
src/kits/package/ActivateRepositoryConfigJob.cpp |  5 ++-
src/kits/package/AddRepositoryRequest.cpp        |  4 +-
src/kits/package/PackageInfoContentHandler.cpp   | 45 +++++++-------------
src/kits/package/PackageRoster.cpp               |  8 +++-
src/kits/package/RefreshRepositoryRequest.cpp    |  6 ++-
src/kits/package/hpkg/PackageFileHeapWriter.cpp  |  4 +-
src/system/kernel/module.cpp                     |  5 +--
src/tools/locale/linkcatkeys.cpp                 | 10 +++--

############################################################################

Commit:      a4a538db9fa6e2195e482c0b95a1be87a74201b2
URL:         https://git.haiku-os.org/haiku/commit/?id=a4a538db9fa6
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Fri Aug  2 23:13:14 2019 UTC

ACPI: Avoid discarding a potential error value.

This is part of a new diagnostic I've managed to wire up
that finds all instances of "status_t" function return
values being discarded using attribute trickery and some
creative Clang pragmas. As you might guess, the resultant
errors list is absolutely gigantic, and most of them are
ones where the failure condition will never (that we care
about) be hit.

The ones in this commit likely are no-op changes, but
they were low-hanging fruit spotted while reviewing
the larger list. The next commit will bring more
substantial changes.

----------------------------------------------------------------------------

diff --git a/src/add-ons/kernel/bus_managers/acpi/ACPICAHaiku.cpp 
b/src/add-ons/kernel/bus_managers/acpi/ACPICAHaiku.cpp
index a65c9166bc..3aad4e020e 100644
--- a/src/add-ons/kernel/bus_managers/acpi/ACPICAHaiku.cpp
+++ b/src/add-ons/kernel/bus_managers/acpi/ACPICAHaiku.cpp
@@ -772,10 +772,9 @@ AcpiOsRemoveInterruptHandler(UINT32 interruptNumber,
        DEBUG_FUNCTION_F("vector: %lu; handler: %p", (uint32)interruptNumber,
                serviceRoutine);
 #ifdef _KERNEL_MODE
-       remove_io_interrupt_handler(interruptNumber,
-               (interrupt_handler) serviceRoutine,
-               sInterruptHandlerData[interruptNumber]);
-       return AE_OK;
+       return remove_io_interrupt_handler(interruptNumber,
+               (interrupt_handler)serviceRoutine,
+               sInterruptHandlerData[interruptNumber]) == B_OK ? AE_OK : 
AE_ERROR;
 #else
        return AE_ERROR;
 #endif
diff --git a/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.h 
b/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.h
index d0149d9c30..d4c71814ab 100644
--- a/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.h
+++ b/src/add-ons/kernel/bus_managers/acpi/EmbeddedController.h
@@ -188,15 +188,13 @@ static status_t
 EcLock(struct acpi_ec_cookie *sc)
 {
        /* If _GLK is non-zero, acquire the global lock. */
-       status_t status = B_OK;
        if (sc->ec_glk) {
-               status = 
sc->ec_acpi_module->acquire_global_lock(EC_LOCK_TIMEOUT,
+               status_t status = 
sc->ec_acpi_module->acquire_global_lock(EC_LOCK_TIMEOUT,
                        &sc->ec_glkhandle);
                if (status != B_OK)
                        return status;
        }
-       mutex_lock(&sc->ec_lock);
-       return status;
+       return mutex_lock(&sc->ec_lock);
 }
 
 

############################################################################

Commit:      dfbf1c8a0ac59206efcf819b420fbbb5bcf55251
URL:         https://git.haiku-os.org/haiku/commit/?id=dfbf1c8a0ac5
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Fri Aug  2 23:14:42 2019 UTC

Package Kit: Avoid discarding potential error values.

Spotted by Clang and the [[nodiscard]] patch.

----------------------------------------------------------------------------

diff --git a/src/kits/package/ActivateRepositoryConfigJob.cpp 
b/src/kits/package/ActivateRepositoryConfigJob.cpp
index f2bf86162c..23ff6f2464 100644
--- a/src/kits/package/ActivateRepositoryConfigJob.cpp
+++ b/src/kits/package/ActivateRepositoryConfigJob.cpp
@@ -46,7 +46,10 @@ ActivateRepositoryConfigJob::Execute()
        if (result != B_OK)
                return result;
 
-       fTargetEntry.SetTo(&fTargetDirectory, repoInfo.Name().String());
+       result = fTargetEntry.SetTo(&fTargetDirectory, 
repoInfo.Name().String());
+       if (result != B_OK)
+               return result;
+
        if (fTargetEntry.Exists()) {
                BString description = BString("A repository configuration for ")
                        << repoInfo.Name() << " already exists.";
diff --git a/src/kits/package/AddRepositoryRequest.cpp 
b/src/kits/package/AddRepositoryRequest.cpp
index 05191b7d31..71378c4a15 100644
--- a/src/kits/package/AddRepositoryRequest.cpp
+++ b/src/kits/package/AddRepositoryRequest.cpp
@@ -77,7 +77,9 @@ AddRepositoryRequest::CreateInitialJobs()
                        tempEntry, fRepositoryBaseURL, targetDirectory);
        if (activateJob == NULL)
                return B_NO_MEMORY;
-       activateJob->AddDependency(fetchJob);
+       result = activateJob->AddDependency(fetchJob);
+       if (result != B_OK)
+               return result;
        if ((result = QueueJob(activateJob)) != B_OK) {
                delete activateJob;
                return result;
diff --git a/src/kits/package/PackageInfoContentHandler.cpp 
b/src/kits/package/PackageInfoContentHandler.cpp
index 62ae30d6d6..0e4ad1d5d1 100644
--- a/src/kits/package/PackageInfoContentHandler.cpp
+++ b/src/kits/package/PackageInfoContentHandler.cpp
@@ -92,44 +92,34 @@ BPackageInfoContentHandler::HandlePackageAttribute(
                        break;
 
                case B_PACKAGE_INFO_COPYRIGHTS:
-                       fPackageInfo.AddCopyright(value.string);
-                       break;
+                       return fPackageInfo.AddCopyright(value.string);
 
                case B_PACKAGE_INFO_LICENSES:
-                       fPackageInfo.AddLicense(value.string);
-                       break;
+                       return fPackageInfo.AddLicense(value.string);
 
                case B_PACKAGE_INFO_PROVIDES:
-                       fPackageInfo.AddProvides(value.resolvable);
-                       break;
+                       return fPackageInfo.AddProvides(value.resolvable);
 
                case B_PACKAGE_INFO_REQUIRES:
-                       fPackageInfo.AddRequires(value.resolvableExpression);
-                       break;
+                       return 
fPackageInfo.AddRequires(value.resolvableExpression);
 
                case B_PACKAGE_INFO_SUPPLEMENTS:
-                       fPackageInfo.AddSupplements(value.resolvableExpression);
-                       break;
+                       return 
fPackageInfo.AddSupplements(value.resolvableExpression);
 
                case B_PACKAGE_INFO_CONFLICTS:
-                       fPackageInfo.AddConflicts(value.resolvableExpression);
-                       break;
+                       return 
fPackageInfo.AddConflicts(value.resolvableExpression);
 
                case B_PACKAGE_INFO_FRESHENS:
-                       fPackageInfo.AddFreshens(value.resolvableExpression);
-                       break;
+                       return 
fPackageInfo.AddFreshens(value.resolvableExpression);
 
                case B_PACKAGE_INFO_REPLACES:
-                       fPackageInfo.AddReplaces(value.string);
-                       break;
+                       return fPackageInfo.AddReplaces(value.string);
 
                case B_PACKAGE_INFO_URLS:
-                       fPackageInfo.AddURL(value.string);
-                       break;
+                       return fPackageInfo.AddURL(value.string);
 
                case B_PACKAGE_INFO_SOURCE_URLS:
-                       fPackageInfo.AddSourceURL(value.string);
-                       break;
+                       return fPackageInfo.AddSourceURL(value.string);
 
                case B_PACKAGE_INFO_CHECKSUM:
                        fPackageInfo.SetChecksum(value.string);
@@ -144,25 +134,20 @@ BPackageInfoContentHandler::HandlePackageAttribute(
                        break;
 
                case B_PACKAGE_INFO_GLOBAL_WRITABLE_FILES:
-                       fPackageInfo.AddGlobalWritableFileInfo(
+                       return fPackageInfo.AddGlobalWritableFileInfo(
                                value.globalWritableFileInfo);
-                       break;
 
                case B_PACKAGE_INFO_USER_SETTINGS_FILES:
-                       
fPackageInfo.AddUserSettingsFileInfo(value.userSettingsFileInfo);
-                       break;
+                       return 
fPackageInfo.AddUserSettingsFileInfo(value.userSettingsFileInfo);
 
                case B_PACKAGE_INFO_USERS:
-                       fPackageInfo.AddUser(value.user);
-                       break;
+                       return fPackageInfo.AddUser(value.user);
 
                case B_PACKAGE_INFO_GROUPS:
-                       fPackageInfo.AddGroup(value.string);
-                       break;
+                       return fPackageInfo.AddGroup(value.string);
 
                case B_PACKAGE_INFO_POST_INSTALL_SCRIPTS:
-                       fPackageInfo.AddPostInstallScript(value.string);
-                       break;
+                       return fPackageInfo.AddPostInstallScript(value.string);
 
                default:
                        if (fErrorOutput != NULL) {
diff --git a/src/kits/package/PackageRoster.cpp 
b/src/kits/package/PackageRoster.cpp
index bae024c657..16e13fbc9d 100644
--- a/src/kits/package/PackageRoster.cpp
+++ b/src/kits/package/PackageRoster.cpp
@@ -181,7 +181,9 @@ BPackageRoster::GetRepositoryCache(const BString& name,
                return result;
        path.Append(name.String());
 
-       repoCacheEntry.SetTo(path.Path());
+       result = repoCacheEntry.SetTo(path.Path());
+       if (result != B_OK)
+               return result;
        return repositoryCache->SetTo(repoCacheEntry);
 }
 
@@ -208,7 +210,9 @@ BPackageRoster::GetRepositoryConfig(const BString& name,
                return result;
        path.Append(name.String());
 
-       repoConfigEntry.SetTo(path.Path());
+       result = repoConfigEntry.SetTo(path.Path());
+       if (result != B_OK)
+               return result;
        return repositoryConfig->SetTo(repoConfigEntry);
 }
 
diff --git a/src/kits/package/RefreshRepositoryRequest.cpp 
b/src/kits/package/RefreshRepositoryRequest.cpp
index 300c1e481e..da23156f9b 100644
--- a/src/kits/package/RefreshRepositoryRequest.cpp
+++ b/src/kits/package/RefreshRepositoryRequest.cpp
@@ -74,7 +74,11 @@ BRefreshRepositoryRequest::CreateInitialJobs()
 
        BRepositoryCache repoCache;
        BPackageRoster roster;
-       roster.GetRepositoryCache(fRepoConfig.Name(), &repoCache);
+       result = roster.GetRepositoryCache(fRepoConfig.Name(), &repoCache);
+       if (result != B_OK) {
+               delete fetchChecksumJob;
+               return result;
+       }
 
        ValidateChecksumJob* validateChecksumJob
                = new (std::nothrow) ValidateChecksumJob(fContext,
diff --git a/src/kits/package/hpkg/PackageFileHeapWriter.cpp 
b/src/kits/package/hpkg/PackageFileHeapWriter.cpp
index 7d1f7acf3f..a86510784c 100644
--- a/src/kits/package/hpkg/PackageFileHeapWriter.cpp
+++ b/src/kits/package/hpkg/PackageFileHeapWriter.cpp
@@ -317,7 +317,9 @@ PackageFileHeapWriter::RemoveDataRanges(
 
        // Before we begin flush any pending data, so we don't need any special
        // handling and also can use the pending data buffer.
-       _FlushPendingData();
+       status_t status = _FlushPendingData();
+       if (status != B_OK)
+               throw status_t(status);
 
        // We potentially have to recompress all data from the first affected 
chunk
        // to the end (minus the removed ranges, of course). As a basic 
algorithm we

############################################################################

Commit:      afb345915dd5da7b40ef8c8303f901786fcb9099
URL:         https://git.haiku-os.org/haiku/commit/?id=afb345915dd5
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Fri Aug  2 23:14:57 2019 UTC

linkcatkeys: Avoid discarding potential error values.

Spotted by Clang and the [[nodiscard]] patch.

----------------------------------------------------------------------------

diff --git a/src/tools/locale/linkcatkeys.cpp b/src/tools/locale/linkcatkeys.cpp
index bb3aec6f28..88fb205bf1 100644
--- a/src/tools/locale/linkcatkeys.cpp
+++ b/src/tools/locale/linkcatkeys.cpp
@@ -127,8 +127,9 @@ main(int argc, char **argv)
                case TARGET_ATTRIBUTE: {
                        BEntry entry(outputFile.String());
                        entry_ref eref;
-                       entry.GetRef(&eref);
-                       res = targetCatImpl.WriteToAttribute(eref);
+                       res = entry.GetRef(&eref);
+                       if (res == B_OK)
+                               res = targetCatImpl.WriteToAttribute(eref);
                        if (res != B_OK) {
                                fprintf(stderr,
                                        "couldn't write target-attribute to %s 
- error: %s\n",
@@ -140,8 +141,9 @@ main(int argc, char **argv)
                case TARGET_RESOURCE: {
                        BEntry entry(outputFile.String());
                        entry_ref eref;
-                       entry.GetRef(&eref);
-                       res = targetCatImpl.WriteToResource(eref);
+                       res = entry.GetRef(&eref);
+                       if (res == B_OK)
+                               res = targetCatImpl.WriteToResource(eref);
                        if (res != B_OK) {
                                fprintf(stderr,
                                        "couldn't write target-resource to %s - 
error: %s\n",

############################################################################

Revision:    hrev53332
Commit:      677e901f80dcaf6f835603825029f8bc5fbd12f7
URL:         https://git.haiku-os.org/haiku/commit/?id=677e901f80dc
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Sat Aug  3 00:13:16 2019 UTC

kernel/module: Utilize RecursiveLocker.

----------------------------------------------------------------------------

diff --git a/src/system/kernel/module.cpp b/src/system/kernel/module.cpp
index 9affc41dba..1c2d2977d3 100644
--- a/src/system/kernel/module.cpp
+++ b/src/system/kernel/module.cpp
@@ -897,7 +897,7 @@ iterator_get_next_module(module_iterator* iterator, char* 
buffer,
        }
 
        if (iterator->loaded_modules) {
-               recursive_lock_lock(&sModulesLock);
+               RecursiveLocker _(sModulesLock);
                ModuleTable::Iterator hashIterator(sModulesHash);
 
                for (int32 i = 0; hashIterator.HasNext(); i++) {
@@ -910,14 +910,11 @@ iterator_get_next_module(module_iterator* iterator, char* 
buffer,
                                        *_bufferSize = strlcpy(buffer, 
module->name, *_bufferSize);
                                        iterator->module_offset = i + 1;
 
-                                       recursive_lock_unlock(&sModulesLock);
                                        return B_OK;
                                }
                        }
                }
 
-               recursive_lock_unlock(&sModulesLock);
-
                // prevent from falling into modules hash iteration again
                iterator->loaded_modules = false;
        }


Other related posts:

  • » [haiku-commits] haiku: hrev53332 - in src: kits/package add-ons/kernel/bus_managers/acpi tools/locale system/kernel kits/package/hpkg - waddlesplash