Re: Comments on the psmisc patch?
On Mon, 2011-10-10 at 23:20 +0200, Svante Signell wrote:
> Hi,
>
> Attached is a patch to make psmisc-22.14 build under GNU/Hurd.
>
> In the patch of fuser.c I rewrote the xreadlink() function, since the
> code in the Alioth patch at
> http://www.gnu.org/software/hurd/hurd/porting/guidelines.html
> does not work :-(
>
> I realize now that setting the error number is not needed for malloc and
> realloc, will change that later.
>
> Functionality is limited, since neither /proc/mounts, /proc/net/unix,
> etc exists.
>
> The last change in fuser.h, not including lists.h jumps over large parts
> of the code, but since the /proc interface is only partially supported
> on Hurd, not much harm is done. An example is: /proc/self/mountinfo
>
> Build and run tested on Hurd and Linux (both __GNU__ and linux versions)
>
> Comments are welcome!
>
> Thanks!
Attached is the latest version of the psmisc patch, from 10 Oct last
year. Below are also the comments from Guillem on 12 Oct:
On Mon, 2011-10-10 at 23:20:15 +0200, Svante Signell wrote:
> In the patch of fuser.c I rewrote the xreadlink() function, since the
> code in the Alioth patch at
> http://www.gnu.org/software/hurd/hurd/porting/guidelines.html
> does not work :-(
I've mentioned this before, for symlinks the ideal solution is to
lstat(2) and then allocate and readlink(2) using st_size + 1.
In this case this does not seem desirable (at least on expandpath)
because the code is trying not to use stat(2) due to NFS issues.
> The last change in fuser.h, not including lists.h jumps over large
parts
> of the code, but since the /proc interface is only partially supported
> on Hurd, not much harm is done. An example is: /proc/self/mountinfo
I think the correct way to fix this is to just partially revert to the
previous upstream code, so that the mountinfo ugliness, _LISTS_H_,
expandpath and all its accompanying PATH_MAX issues get conditionalized
for Linux systems only. There's no point in all that code on other
systems really, and more so when upstream has conditionalized and
disabled by default the timeout fork stat (for the NFS issue) on
latest git.
thanks,
guillem
Any ideas on what to change/improve before submitting a bug report?
Svante
diff -ur psmisc-22.14/debian/rules psmisc-22.14.modified//debian/rules
--- psmisc-22.14/debian/rules 2011-09-14 18:40:34.000000000 +0200
+++ psmisc-22.14.modified//debian/rules 2011-09-21 13:03:45.000000000 +0200
@@ -7,6 +7,6 @@
override_dh_auto_install:
dh_auto_install
mv debian/psmisc/usr/bin/fuser debian/psmisc/bin/fuser
- if [ "$(DEB_HOST_ARCH_OS)" = "kfreebsd" ] ; then \
+ if [ "$(DEB_HOST_ARCH_OS)" = "kfreebsd" -o "$(DEB_HOST_ARCH_OS)" = "hurd" ] ; then \
rm debian/psmisc/usr/share/man/man1/peekfd.1 ; \
fi
diff -ur psmisc-22.14/src/fuser.c psmisc-22.14.modified//src/fuser.c
--- psmisc-22.14/src/fuser.c 2011-06-20 13:41:13.000000000 +0200
+++ psmisc-22.14.modified//src/fuser.c 2011-10-10 22:28:59.000000000 +0200
@@ -1951,10 +1951,40 @@
* Somehow the realpath(3) glibc function call, nevertheless
* it avoids lstat(2) system calls.
*/
+#ifndef __GNU__
static char real[PATH_MAX+1];
+#else
+char* xreadlink(const char *filename) {
+ int nchars, size = 128;
+ char *buffer;
+ if ((buffer=(char*)malloc(size)) == NULL) {
+ errno = ENOMEM;
+ return NULL;
+ }
+ while ((nchars = readlink(filename,buffer,size)) == size) {
+ size *= 2;
+ if ((buffer = (char*)realloc(buffer,size)) == NULL) {
+ errno = ENOMEM;
+ return NULL;
+ }
+ }
+ if (nchars < 0) {
+ free (buffer);
+ errno = EINVAL;
+ return NULL;
+ }
+ buffer[nchars]='\0';
+ return buffer;
+}
+#endif
+
char* expandpath(const char * path)
{
+#ifndef __GNU__
char tmpbuf[PATH_MAX+1];
+#else
+ char *tmpbuf=NULL;
+#endif
const char *start, *end;
char *curr, *dest;
int deep = MAXSYMLINKS;
@@ -1962,6 +1992,7 @@
if (!path || *path == '\0')
return (char*)0;
+#ifndef __GNU__
curr = &real[0];
if (*path != '/') {
@@ -1972,6 +2003,17 @@
*curr = '/';
dest = curr + 1;
}
+#else
+ if (*path != '/') {
+ if ((curr = get_current_dir_name()) == NULL)
+ return (char*)0;
+ dest = rawmemchr(curr, '\0');
+ } else {
+ curr = malloc(1 + 1);
+ *curr = '/';
+ dest = curr + 1;
+ }
+#endif
for (start = end = path; *start; start = end) {
@@ -1990,27 +2032,37 @@
while ((--dest)[-1] != '/')
;
} else {
+#ifndef __GNU__
char lnkbuf[PATH_MAX+1];
+#else
+ char *lnkbuf = NULL;
+#endif
size_t len;
ssize_t n;
if (dest[-1] != '/')
*dest++ = '/';
+#ifndef __GNU__
if (dest + (end - start) > curr + PATH_MAX) {
errno = ENAMETOOLONG;
return (char*)0;
}
+#endif
dest = mempcpy(dest, start, end - start);
*dest = '\0';
if (deep-- < 0) {
errno = ELOOP;
+#ifdef __GNU__
+ free(curr);
+#endif
return (char*)0;
}
errno = 0;
+#ifndef __GNU__
if ((n = readlink(curr, lnkbuf, PATH_MAX)) < 0) {
deep = MAXSYMLINKS;
if (errno == EINVAL)
@@ -2024,6 +2076,21 @@
errno = ENAMETOOLONG;
return (char*)0;
}
+#else
+ lnkbuf = xreadlink(curr);
+ if (lnkbuf == NULL) {
+ deep = MAXSYMLINKS;
+ if (errno == EINVAL)
+ continue; /* Not a symlink */
+ free(lnkbuf);
+ free(curr);
+ return (char*)0;
+ }
+
+ len = strlen(end);
+ n = strlen(lnkbuf);
+ tmpbuf = malloc(n + len + 1);
+#endif
memmove(&tmpbuf[n], end, len + 1);
path = end = memcpy(tmpbuf, lnkbuf, n);
@@ -2033,6 +2100,9 @@
else if (dest > curr + 1)
while ((--dest)[-1] != '/');
+#ifdef __GNU__
+ free(lnkbuf);
+#endif
}
}
@@ -2040,5 +2110,8 @@
--dest;
*dest = '\0';
+#ifdef __GNU__
+ free(tmpbuf);
+#endif
return curr;
}
diff -ur psmisc-22.14/src/fuser.h psmisc-22.14.modified//src/fuser.h
--- psmisc-22.14/src/fuser.h 2011-02-09 23:53:26.000000000 +0100
+++ psmisc-22.14.modified//src/fuser.h 2011-10-04 13:54:11.000000000 +0200
@@ -86,7 +86,7 @@
struct mount_list *next;
};
-#if defined (__GNUC__) && defined(__OPTIMIZE__)
+#if defined (__GNUC__) && defined(__OPTIMIZE__) && !defined(__GNU__)
# include "lists.h"
typedef struct mntinfo_s {
list_t this;
Reply to: