View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001626 | GNUnet | obsolete | public | 2010-12-01 06:58 | 2024-05-03 13:51 |
| Reporter | LRN | Assigned To | LRN | ||
| Priority | normal | Severity | trivial | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Target Version | Git master | ||||
| Summary | 0001626: Use IPK_SELF_PREFIX to form a path to peerinfo service | ||||
| Description | This is a side-effect of the way GNUNET_OS_start_process is implemented on different platforms. On *nix it uses execvp() to locate the executable binary, and it is able to find it in working directory (probably because working directory is in PATH, or because execvp is written this way, i do not know). On W32 it uses CreateProcess, which always takes a full path and is incapable of locating the executable in any way. ndurner have pointed out that full paths should be passed to GNUNET_OS_start_process() anyway, so he's not going to fix executable file location in GNUNET_OS_start_process on W32. The patch constructs a full path to gnunet-service-peerinfo instead of running just "gnunet-service-peerinfo" during the peerinfo tests. | ||||
| Tags | No tags attached. | ||||
| Attached Files | 0001-Use-IPK_SELF_PREFIX-to-form-a-path-to-peerinfo-servi.patch (2,273 bytes)
From b1616133a31b6396821d11812e234c64c3ef25e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D0=A0=D1=83=D1=81=D0=BB=D0=B0=D0=BD=20=D0=98=D0=B6=D0=B1=D1=83=D0=BB=D0=B0=D1=82=D0=BE=D0=B2?= <lrn1986@gmail.com>
Date: Wed, 1 Dec 2010 08:50:27 +0300
Subject: [PATCH] Use IPK_SELF_PREFIX to form a path to peerinfo service
---
src/peerinfo/perf_peerinfo_api.c | 9 +++++++--
src/peerinfo/test_peerinfo_api.c | 9 +++++++--
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/peerinfo/perf_peerinfo_api.c b/src/peerinfo/perf_peerinfo_api.c
index fe1851a..cac7b9e 100755
--- a/src/peerinfo/perf_peerinfo_api.c
+++ b/src/peerinfo/perf_peerinfo_api.c
@@ -168,8 +168,13 @@ check ()
struct GNUNET_GETOPT_CommandLineOption options[] = {
GNUNET_GETOPT_OPTION_END
};
- proc = GNUNET_OS_start_process (NULL, NULL, "gnunet-service-peerinfo",
- "gnunet-service-peerinfo",
+ char *gnunet_service_peerinfo;
+ char *self_dir = GNUNET_OS_installation_get_path (GNUNET_OS_IPK_SELF_PREFIX);
+ GNUNET_asprintf (&gnunet_service_peerinfo, "%s%s", self_dir, "gnunet-service-peerinfo");
+ GNUNET_free (self_dir);
+
+ proc = GNUNET_OS_start_process (NULL, NULL, gnunet_service_peerinfo,
+ gnunet_service_peerinfo,
#if DEBUG_PEERINFO
"-L", "DEBUG",
#else
diff --git a/src/peerinfo/test_peerinfo_api.c b/src/peerinfo/test_peerinfo_api.c
index fdd0dc4..62cb3df 100644
--- a/src/peerinfo/test_peerinfo_api.c
+++ b/src/peerinfo/test_peerinfo_api.c
@@ -173,8 +173,13 @@ check ()
struct GNUNET_GETOPT_CommandLineOption options[] = {
GNUNET_GETOPT_OPTION_END
};
- proc = GNUNET_OS_start_process (NULL, NULL, "gnunet-service-peerinfo",
- "gnunet-service-peerinfo",
+ char *gnunet_service_peerinfo;
+ char *self_dir = GNUNET_OS_installation_get_path (GNUNET_OS_IPK_SELF_PREFIX);
+ GNUNET_asprintf (&gnunet_service_peerinfo, "%s%s", self_dir, "gnunet-service-peerinfo");
+ GNUNET_free (self_dir);
+
+ proc = GNUNET_OS_start_process (NULL, NULL, gnunet_service_peerinfo,
+ gnunet_service_peerinfo,
#if DEBUG_PEERINFO
"-L", "DEBUG",
#endif
--
1.7.3.1.msysgit.0
| ||||
|
|
I'm not sure I like the idea of having to construct full paths everywhere we call start_process. This seems like bad API design, especially since it is a common requirement to have the full path. I do not see why GNUNET_OS_start_process should not do the work to construct the full path if necessary. What was ndurner's reasoning? |
|
|
My original proposal was to perform a search for the executable file (if the path given to GNUNET_OS_start_process is not absolute). I've planned to implement a lookup function that would, depending on its arguments, look in: system directories PATH directories working directory user-supplied list of directories in user-supplied order (with reasonable defaults if user passes NULL insteadf of a string that specifies the order), such as "spuw" - system, PATH, user-supplied, workdir - or "su" - only system or user-supplied list of directories, possibly looking also for the executable file with different extensions, with list of possible extensions supplied by the user (how to run non-exe files is yet another matter, since CreateProcess only runs binary executables) ndurner claimed that: 1) Such function would require a lot of KLOC and he's not going to commit that 2) Looking for executables in working directory is a security risk My counter-reasoning for this is: 1) We can use WinAPI to perform a search, but WinAPI offers much less control over the search procedure than my own implementation would (see also (2)) 2) My function will not look for files in working directory, unless it is asked to do so (and GNUNET_OS_start_process wouldn't ask for that). To think of it, i've never mentioned that the directory from which current module was loaded should be searched. Maybe that would have placated ndurner, i don't know. |
|
|
Well, I don't think we need to do a complex function as you propose; just searching $PATH should do. Or, if something similar exists, use the respective Windows API that is recommended. I agree with ndurner that we should not (1) add a ton of code or (2) search the working directory, but that does not mean that we need to pass an absolute path; all we need is to use a standard "find executable" function from Windows API (or a simple, $PATH-based lookup if there is no W32-API function) to do this. |
|
|
I've updated 0001616 , it's somewhat related (i didn't test it with peerinfo, but it should work) |
|
|
Is this issue still active? |
|
|
AFAIR, the changes that were eventually introduced into process-spawning function resulted in this issue resolving itself. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2010-12-01 06:58 | LRN | New Issue | |
| 2010-12-01 06:58 | LRN | File Added: 0001-Use-IPK_SELF_PREFIX-to-form-a-path-to-peerinfo-servi.patch | |
| 2010-12-02 08:40 | Christian Grothoff | Note Added: 0004193 | |
| 2010-12-02 08:40 | Christian Grothoff | Status | new => feedback |
| 2010-12-02 10:40 | LRN | Note Added: 0004194 | |
| 2010-12-05 21:58 | Christian Grothoff | Note Added: 0004200 | |
| 2010-12-05 22:08 | LRN | Note Added: 0004202 | |
| 2010-12-27 00:40 | NDurner | Relationship added | related to 0001616 |
| 2010-12-27 00:40 | NDurner | Status | feedback => assigned |
| 2010-12-27 00:40 | NDurner | Assigned To | => NDurner |
| 2011-04-28 19:33 | Christian Grothoff | Category | peerinfo service => Win32 port |
| 2011-06-23 12:36 | Christian Grothoff | Note Added: 0004461 | |
| 2011-06-23 12:36 | Christian Grothoff | Status | assigned => feedback |
| 2011-06-23 14:28 | LRN | Note Added: 0004464 | |
| 2011-09-15 19:49 | Christian Grothoff | Status | feedback => resolved |
| 2011-09-15 19:49 | Christian Grothoff | Resolution | open => fixed |
| 2011-09-15 19:49 | Christian Grothoff | Assigned To | NDurner => LRN |
| 2011-09-18 15:00 | Christian Grothoff | Status | resolved => closed |
| 2024-01-12 14:28 | schanzen | Category | Win32 port => Win32 port (deprecated) |
| 2024-05-03 13:51 | Christian Grothoff | Category | Win32 port (deprecated) => obsolete |
