Skip to content

Commit

Permalink
configuration: improve handling of positional arguments
Browse files Browse the repository at this point in the history
Currently getopt_long is configured to support mixing options and
positional arguments, but kcov don't use the special "--" argument to
tell getopt_long when to stop parsing the executable arguments.
Instead the implementation tries to scan the arguments to find an
executable, but the implementation is flawed because there are two
required positional arguments, and both may be executable.

In order to handle positional arguments correctly, configure getopt_long
to stop parsing after encountering a positional argument.

This change greatly simplified the code, since the pre scanning can be
removed.  However it may break the workflow of some users.

Add a regression test for issue #414

Issue #414: kcov: error when out-dir is an executable
  • Loading branch information
perillo committed Mar 15, 2024
1 parent 692d41c commit 989f293
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 43 deletions.
55 changes: 12 additions & 43 deletions src/configuration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,6 @@ using namespace kcov;

extern "C" const char *kcov_version;

// This function can't be defined in utils.cc due to the use of the singleton
// pattern. It will cause a linker error when buildind tests, since the utils.cc
// file is also included in the test source files.
static bool is_supported_executable(const std::string path)
{
return kcov::IParserManager::getInstance().matchParser(path) != 0;
}

class Configuration : public IConfiguration
{
public:
Expand Down Expand Up @@ -170,41 +162,12 @@ class Configuration : public IConfiguration
{ 0, 0, 0, 0 } };
unsigned int afterOpts = 0;
unsigned int extraNeeded = 2;
unsigned int lastArg;
bool printUsage = false;

setupDefaults();

setKey("kcov-binary-path", get_real_path(argv[0]));

/* Scan through the parameters for a supported executable file: That
* will be the second last argument in the list.
*
* After that it's arguments to the external program.
*/
for (lastArg = 1; lastArg < argc; lastArg++)
{
std::string arg = std::string(argv[lastArg]);
int ret;
std::string path;

// Skip options.
if ((arg.size() > 0) && (arg[0] == '-'))
continue;

ret = look_path(arg, &path);
if (ret < 0)
continue;

if (is_supported_executable(path))
{
// Intentional memory leak
argv[lastArg] = xstrdup(path.c_str());
break;
}
}

bool printUsage = false;

/* Hooray for reentrancy... */
optind = 0;
optarg = 0;
Expand All @@ -213,7 +176,11 @@ class Configuration : public IConfiguration
int option_index = 0;
int c;

c = getopt_long(lastArg, (char **) argv, "hp:s:l:t:", long_options,
// Force getopt_long to stop as soon as a nonoption argument is
// encountered. This will ensure correct parsing of positional
// arguments without having to use "--" to stop parsing the
// executable arguments.
c = getopt_long(argc, (char **) argv, "+hp:s:l:t:", long_options,
&option_index);

/* No more options */
Expand Down Expand Up @@ -492,7 +459,6 @@ class Configuration : public IConfiguration
}
else
{
std::string binaryName;
std::string path;

if (keyAsInt("attach-pid") == 0)
Expand All @@ -513,8 +479,11 @@ class Configuration : public IConfiguration
}
}

path = get_real_path(path);
std::pair<std::string, std::string> tmp = split_path(path);
std::string binaryName;
std::string binaryPath = path;

(void)look_path(path, &binaryPath);
std::pair<std::string, std::string> tmp = split_path(binaryPath);

setKey("binary-path", tmp.first);
binaryName = tmp.second;
Expand All @@ -523,7 +492,7 @@ class Configuration : public IConfiguration
{
setKey("target-directory",
fmt("%s/%s.%08zx", outDirectory.c_str(), binaryName.c_str(),
(size_t)hash_file(path)));
(size_t)hash_file(get_real_path(binaryPath))));
}
else
{
Expand Down
8 changes: 8 additions & 0 deletions tests/tools/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,11 @@ def runTest(self):
dom = parse_cobertura.parseFile(testbase.outbase + "/kcov/main/cobertura.xml")
assert parse_cobertura.hitsPerLine(dom, "second.py", 34) == 2
assert noKcovRv, rv

# Issue #414
class OutDirectoryIsExecutable(testbase.KcovTestCase):
def runTest(self):
self.setUp()
rv,o = self.do(testbase.kcov + " echo python -V")

assert rv == 0

0 comments on commit 989f293

Please sign in to comment.