Skip to content

Commit

Permalink
Eliminate uses of realpath() and rpmCleanPath() in fingerprint canonD…
Browse files Browse the repository at this point in the history
…ir()

Use filesystem::canonical() instead of realpath() and the new
rpm::join_path() and rpm::normalize_path() for the other path
manipulation. Also return a C++ string to the sole caller.

Drop some bogus checks and comments: canonDir() could not return NULL
for a long long time.

One could argue that there's more than one change here but going from C
strings to C++ strings tends to have a bit of a domino effect.

No functional changes intended.
  • Loading branch information
pmatilai committed Nov 12, 2024
1 parent 01ede27 commit 533d836
Showing 1 changed file with 19 additions and 19 deletions.
38 changes: 19 additions & 19 deletions lib/fprint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,19 @@

#include "system.h"

#include <filesystem>
#include <system_error>
#include <unordered_map>

#include <rpm/rpmfileutil.h> /* for rpmCleanPath */
#include <rpm/rpmfileutil.h>
#include <rpm/rpmstring.h>
#include <rpm/rpmts.h>
#include <rpm/rpmsq.h>

#include "rpmdb_internal.hh"
#include "rpmfi_internal.hh"
#include "rpmte_internal.hh"
#include "rpmmacro_internal.hh"
#include "fprint.hh"
#include "misc.hh"
#include "debug.h"
Expand Down Expand Up @@ -134,32 +137,31 @@ static const struct fprintCacheEntry_s * cacheContainsDirectory(
return NULL;
}

static char * canonDir(rpmstrPool pool, rpmsid dirNameId)
static std::string canonDir(rpmstrPool pool, rpmsid dirNameId)
{
const char * dirName = rpmstrPoolStr(pool, dirNameId);
char *cdnbuf = NULL;
std::string cdnbuf;

if (*dirName == '/') {
cdnbuf = xstrdup(dirName);
cdnbuf = dirName;
} else {
/* Using realpath on the arg isn't correct if the arg is a symlink,
/* Using canonical on the arg isn't correct if the arg is a symlink,
* especially if the symlink is a dangling link. What we
* do instead is use realpath() on `.' and then append arg to
* do instead is use canonical() on `.' and then append arg to
* the result.
*/

/* if the current directory doesn't exist, we might fail. oh well. */
if ((cdnbuf = realpath(".", NULL)) != NULL)
cdnbuf = rstrscat(&cdnbuf, "/", dirName, NULL);
std::error_code ec {};
cdnbuf = std::filesystem::canonical(".", ec);
if (ec)
cdnbuf += rpm::join_path({cdnbuf, dirName}, false);
}

/* ensure canonical path with a trailing slash */
if (cdnbuf) {
size_t cdnl = strlen(cdnbuf);
if (cdnl > 1) {
cdnbuf = rpmCleanPath(cdnbuf);
cdnbuf = rstrcat(&cdnbuf, "/");
}
if (cdnbuf.empty() == false) {
cdnbuf = rpm::normalize_path(cdnbuf);
cdnbuf += '/';
}
return cdnbuf;
}
Expand All @@ -171,12 +173,11 @@ static int doLookupId(fingerPrintCache cache,
{
struct stat sb;
const struct fprintCacheEntry_s * cacheHit;
char *cdn = canonDir(cache->pool, dirNameId);
std::string dir = canonDir(cache->pool, dirNameId);
const char *cdn = dir.c_str();
rpmsid fpId;
size_t fpLen;

if (cdn == NULL) goto exit; /* XXX only if realpath() above fails */

memset(fp, 0, sizeof(*fp));
fpId = rpmstrPoolId(cache->pool, cdn, 1);
fpLen = rpmstrPoolStrlen(cache->pool, fpId);;
Expand Down Expand Up @@ -219,8 +220,7 @@ static int doLookupId(fingerPrintCache cache,
}

exit:
free(cdn);
/* XXX TODO: failure from eg realpath() should be returned and handled */
/* XXX TODO: failure from eg canonical() should be returned and handled */
return 0;
}

Expand Down

0 comments on commit 533d836

Please sign in to comment.