diff --git a/CHANGELOG.md b/CHANGELOG.md --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## 0.3.5.1 - 2024-05-03 + +### Fixed + + - Remove most uses of `c_str()`, of which at least one exhibited undefined behavior. + ## 0.3.5.0 - 2024-02-12 ### Added diff --git a/cbits/string.cxx b/cbits/string.cxx new file mode 100644 --- /dev/null +++ b/cbits/string.cxx @@ -0,0 +1,12 @@ +#include "hercules-ci-cnix/string.hxx" + +namespace hercules_ci_cnix { + +char * stringdup(const std::string & s) { + char * p = (char *)malloc(s.size() + 1); + std::copy(s.begin(), s.end(), p); + p[s.size()] = '\0'; + return p; +} + +} // namespace hercules_ci_cnix diff --git a/hercules-ci-cnix-store.cabal b/hercules-ci-cnix-store.cabal --- a/hercules-ci-cnix-store.cabal +++ b/hercules-ci-cnix-store.cabal @@ -1,7 +1,7 @@ cabal-version: 2.4 name: hercules-ci-cnix-store -version: 0.3.5.0 +version: 0.3.5.1 synopsis: Haskell bindings for Nix's libstore category: Nix homepage: https://docs.hercules-ci.com @@ -43,6 +43,8 @@ cxx-options: -Wall + -- GHC does _not_ produce declarations, so this only applies to cxx-sources. + -Werror=missing-declarations extra-libraries: stdc++ if os(darwin) @@ -96,7 +98,10 @@ , nix-main >= 2.4 && < 2.19 || (>= 2.19.3 && < 2.21) install-includes: hercules-ci-cnix/store.hxx + hercules-ci-cnix/string.hxx hs-source-dirs: src + cxx-sources: + cbits/string.cxx build-depends: base >= 4.7 && <5 , inline-c diff --git a/include/hercules-ci-cnix/string.hxx b/include/hercules-ci-cnix/string.hxx new file mode 100644 --- /dev/null +++ b/include/hercules-ci-cnix/string.hxx @@ -0,0 +1,12 @@ +#pragma once +#include + +namespace hercules_ci_cnix { + +/** + c_str() on a std::string only works if the std::string doesn't go out of scope. + `stringdup` is like `strdup(s.c_str())`, but without the undefined behavior. + */ +char * stringdup(const std::string & s); + +} // namespace hercules_ci_cnix diff --git a/src/Hercules/CNix.hs b/src/Hercules/CNix.hs --- a/src/Hercules/CNix.hs +++ b/src/Hercules/CNix.hs @@ -59,8 +59,12 @@ C.include "" +C.include "hercules-ci-cnix/string.hxx" + C.using "namespace nix" +C.using "namespace hercules_ci_cnix" + init :: IO () init = void @@ -107,7 +111,7 @@ nixVersion = unsafePerformIO $ do p <- [C.exp| const char* { - strdup(nix::nixVersion.c_str()) + stringdup(nix::nixVersion) }|] unsafePackMallocCString p {-# NOINLINE nixVersion #-} diff --git a/src/Hercules/CNix/Settings.hs b/src/Hercules/CNix/Settings.hs --- a/src/Hercules/CNix/Settings.hs +++ b/src/Hercules/CNix/Settings.hs @@ -40,7 +40,10 @@ C.include "" C.include "" C.include "" +C.include "hercules-ci-cnix/string.hxx" +C.using "namespace hercules_ci_cnix" + byteStringSet :: IO (Ptr (Std.Set.CStdSet Std.String.CStdString)) -> IO (Set ByteString) byteStringSet x = x @@ -67,7 +70,7 @@ getSystem = unsafePackMallocCString =<< [C.exp| const char *{ - strdup(nix::settings.thisSystem.get().c_str()) + stringdup(nix::settings.thisSystem.get()) }|] getSystemFeatures :: IO (Set ByteString) @@ -107,7 +110,7 @@ getNetrcFile = unsafePackMallocCString =<< [C.exp| const char *{ - strdup(nix::settings.netrcFile.get().c_str()) + stringdup(nix::settings.netrcFile.get()) }|] -- Gets the value of https://nixos.org/manual/nix/stable/command-ref/conf-file.html?highlight=use-sqlite-wal#conf-use-sqlite-wal diff --git a/src/Hercules/CNix/Store.hs b/src/Hercules/CNix/Store.hs --- a/src/Hercules/CNix/Store.hs +++ b/src/Hercules/CNix/Store.hs @@ -82,6 +82,8 @@ C.include "hercules-ci-cnix/store.hxx" +C.include "hercules-ci-cnix/string.hxx" + #if NIX_IS_AT_LEAST(2,19,0) C.include "" @@ -98,6 +100,8 @@ C.using "namespace nix" +C.using "namespace hercules_ci_cnix" + forNonNull :: Applicative m => Ptr a -> (Ptr a -> m b) -> m (Maybe b) forNonNull = flip traverseNonNull @@ -150,7 +154,7 @@ unsafeMallocBS [C.block| const char* { std::string uri = (*$(refStore* store))->getUri(); - return strdup(uri.c_str()); + return stringdup(uri); } |] -- | Usually @"/nix/store"@ @@ -159,7 +163,7 @@ unsafeMallocBS [C.block| const char* { std::string uri = (*$(refStore* store))->storeDir; - return strdup(uri.c_str()); + return stringdup(uri); } |] getStoreProtocolVersion :: Store -> IO Int @@ -209,7 +213,7 @@ BS.unsafePackMallocCString =<< [C.block| const char* { std::string s($fptr-ptr:(nix::StorePath *storePath)->to_string()); - return strdup(s.c_str()); + return stringdup(s); }|] pure $ toS $ decodeUtf8With lenientDecode bs @@ -250,7 +254,7 @@ BS.unsafePackMallocCString =<< [C.block| const char *{ std::string s($fptr-ptr:(nix::StorePath *sp)->to_string()); - return strdup(s.c_str()); + return stringdup(s); }|] getStorePathHash :: StorePath -> IO ByteString @@ -258,7 +262,7 @@ BS.unsafePackMallocCString =<< [C.block| const char *{ std::string s($fptr-ptr:(nix::StorePath *sp)->hashPart()); - return strdup(s.c_str()); + return stringdup(s); }|] storePathToPath :: Store -> StorePath -> IO ByteString @@ -268,7 +272,7 @@ Store & store = **$(refStore* store); StorePath &sp = *$fptr-ptr:(nix::StorePath *sp); std::string s(store.printStorePath(sp)); - return strdup(s.c_str()); + return stringdup(s); }|] ensurePath :: Store -> StorePath -> IO () @@ -409,7 +413,7 @@ =<< [C.throwBlock| const char *{ StorePath &sp = *$fptr-ptr:(nix::StorePath *storePath); std::string s(Derivation::nameFromPath(sp)); - return strdup(s.c_str()); + return stringdup(s); }|] data DerivationOutput = DerivationOutput @@ -469,7 +473,7 @@ int &hashSize = *$(int *hashSizeP); std::string nameString = i->first; - name = strdup(nameString.c_str()); + name = stringdup(nameString); path = nullptr; std::visit(overloaded { #if NIX_IS_AT_LEAST(2, 18, 0) @@ -797,14 +801,14 @@ getDerivationPlatform derivation = unsafeMallocBS [C.exp| const char* { - strdup($fptr-ptr:(Derivation *derivation)->platform.c_str()) + stringdup($fptr-ptr:(Derivation *derivation)->platform) } |] getDerivationBuilder :: Derivation -> IO ByteString getDerivationBuilder derivation = unsafeMallocBS [C.exp| const char* { - strdup($fptr-ptr:(Derivation *derivation)->builder.c_str()) + stringdup($fptr-ptr:(Derivation *derivation)->builder) } |] getDerivationArguments :: Derivation -> IO [ByteString] @@ -957,7 +961,7 @@ if isEnd then pure [] else do - s <- [C.exp| const char*{ strdup((*$(StringsIterator *i))->c_str()) }|] + s <- [C.exp| const char*{ stringdup(*(*$(StringsIterator *i))) }|] bs <- BS.unsafePackMallocCString s [C.block| void { (*$(StringsIterator *i))++; }|] (bs :) <$> go @@ -971,8 +975,8 @@ if isEnd then pure [] else do - k <- [C.exp| const char*{ strdup((*$(StringPairsIterator *i))->first.c_str()) }|] - v <- [C.exp| const char*{ strdup((*$(StringPairsIterator *i))->second.c_str()) }|] + k <- [C.exp| const char*{ stringdup((*$(StringPairsIterator *i))->first) }|] + v <- [C.exp| const char*{ stringdup((*$(StringPairsIterator *i))->second) }|] bk <- BS.unsafePackMallocCString k bv <- BS.unsafePackMallocCString v [C.block| void { (*$(StringPairsIterator *i))++; }|] @@ -1209,7 +1213,7 @@ #else std::string s((*$fptr-ptr:(refValidPathInfo* vpi))->narHash.to_string(nix::Base32, true)); #endif - return strdup(s.c_str()); + return stringdup(s); }|] -- | Deriver field of a ValidPathInfo struct. Source: store-api.hh