Re: [PATCH 1/5] bb_socklen_t must be signed int
Is there any discussion on this one? It should be pretty straightforward. Without this fix, we get warnings such as: networking/ftpd.c: In function 'bind_for_passive_mode': networking/ftpd.c:126:21: warning: pointer targets in passing argument 3 of 'getsockname' differ in signedness [-Wpointer-sign] #define G (*(struct globals*)&bb_common_bufsiz1) ^ networking/ftpd.c:438:40: note: in expansion of macro 'G' getsockname(fd, &G.local_addr->u.sa, &G.local_addr->len); ^ In file included from /usr/local/arm-linux-androideabi/sysroot/usr/include/netdb.h:66:0, from include/libbb.h:20, from networking/ftpd.c:31: /usr/local/arm-linux-androideabi/sysroot/usr/include/sys/socket.h:68:18: note: expected 'socklen_t *' but argument is of type 'bb_socklen_t *' __socketcall int getsockname(int, struct sockaddr *, socklen_t *); ^ On Friday, 24 April 2015, at 4:12 am, Matt Whitlock wrote: > BSD and POSIX specify socklen_t as int. Busybox should too, to avoid pointer > signedness warnings. > --- > include/libbb.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/libbb.h b/include/libbb.h > index f0ac1f5..dc0acd8 100644 > --- a/include/libbb.h > +++ b/include/libbb.h > @@ -143,7 +143,7 @@ > * (in case "is it defined already" detection above failed) > */ > # define socklen_t bb_socklen_t > - typedef unsigned socklen_t; > + typedef int socklen_t; > # endif > #endif > #ifndef HAVE_CLEARENV > -- > 2.0.5 > ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Bionic lacks transparent LFS migrations; provide a workaround
On Sunday, 3 May 2015, at 6:54 pm, Denys Vlasenko wrote: > On Sat, Apr 25, 2015 at 11:33 PM, Matt Whitlock > wrote: > > On Saturday, 25 April 2015, at 7:17 pm, Denys Vlasenko wrote: > >> Let me know how I can test this on a x86 machine. > > > > Bionic doesn't really support vanilla Linux. > > Do you really build your busybox on your phone? > > I guess you have a cross-compiler or something. Oh, sorry for the misunderstanding. I thought you wanted to test on an x86 machine. Yes, I do build BusyBox on x86_64. I use the Android NDK cross-compiler. > Where can I get it to do my own builds? https://developer.android.com/tools/sdk/ndk/index.html See the documentation for the make-standalone-toolchain.sh script. I invoked it as follows to set up the toolchain that I use for building BusyBox: # ./make-standalone-toolchain.sh --ndk-dir=/opt/android-ndk --system=linux-x86_64 --arch=arm --platform=android-15 --install-dir=/usr/local/arm-linux-androideabi > As to the patch, I can take it. Can you add a comment on top > of the added code which version of Bionic it was tested with, > what are the quirks (e.g. "struct stat having 64-bit st_ino > and st_size" are quirks, normal C libc don't do that...). I'm going to write some test cases to exercise all of the migrated types (off_t, ino_t, blkcnt_t, fsblkcnt_t, fsfilcnt_t) on ARM Android, and I will test building against various versions of Bionic and executing on KitKat and Lollipop. I'll get back to you with my findings. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 2/5] Bionic lacks mempcpy; enable existing workaround
(The attached patch subsumes the previous "Bionic lacks mempcpy; enable existing workaround".) Bionic has been growing new features over the years, so platform.h should test the __ANDROID_API__ macro to determine which functions to polyfill. Specifically: * Bionic has supported fdprintf() since API level 8. This is a direct replacement for dprintf(). (The Bionic header chides Glibc for choosing the name "dprintf".) * Bionic has supported ttyname_r(), getline(), and stpcpy() since API level 21. >From 3c0effb4ccab30fb59d70d5837ad46b50a196ef0 Mon Sep 17 00:00:00 2001 From: Matt Whitlock Date: Tue, 28 Apr 2015 22:37:42 -0400 Subject: [PATCH] conditionally undefine feature macros depending on Android API level Also, undefine HAVE_MEMPCPY on Android, as Bionic lacks this function in all released versions (but upstream master does have it, so support is coming). --- include/platform.h | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/include/platform.h b/include/platform.h index d5ab7bc..1706b18 100644 --- a/include/platform.h +++ b/include/platform.h @@ -480,10 +480,17 @@ typedef unsigned smalluint; #endif #if defined(ANDROID) || defined(__ANDROID__) -# undef HAVE_DPRINTF -# undef HAVE_TTYNAME_R -# undef HAVE_GETLINE -# undef HAVE_STPCPY +# if __ANDROID_API__ < 8 +# undef HAVE_DPRINTF +# else +# define dprintf fdprintf +# endif +# if __ANDROID_API__ < 21 +# undef HAVE_TTYNAME_R +# undef HAVE_GETLINE +# undef HAVE_STPCPY +# endif +# undef HAVE_MEMPCPY # undef HAVE_STRCHRNUL # undef HAVE_STRVERSCMP # undef HAVE_UNLOCKED_LINE_OPS -- 2.3.6 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Bionic lacks transparent LFS migrations; provide a workaround
On Wednesday, 29 April 2015, at 1:26 am, Tanguy Pruvot wrote:
> For this case i generally cast the param to the biggest possible type.
>
> printf("llu", (uint64_t) val);
You probably shouldn't assume that uint64_t is the largest possible integer
type. In particular, off_t is not guaranteed to be <= 64 bits.
#include
printf("%"PRIuMAX, (uintmax_t) val);
But I hate the idea of involving all the nasty large-integer arithmetic
machinery if it's not strictly necessary.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Bionic lacks transparent LFS migrations; provide a workaround
On Tuesday, 28 April 2015, at 3:55 pm, Matt Whitlock wrote: > I'll work on finishing the Large File Support in Bionic and adding support > for _FILE_OFFSET_BITS==64. The Bionic devs have already finished Large File Support, including support for _FILE_OFFSET_BITS==64, in the latest Bionic, so there's nothing further to contribute upstream. If you don't want BusyBox to support large files on older versions of Bionic, I can live with that. My patch works just fine for my purposes. Would you consider a patch to BusyBox that adds an explicit cast to off_t wherever values of st_size are used with the OFF_FMT format specifier? Without such a cast, when compiling without LFS on Bionic, we get warnings like: archival/tar.c: In function 'writeTarHeader': archival/tar.c:448:5: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'long long int' [-Wformat=] fileName, statbuf->st_size); ^ I don't know if GCC is smart enough to cast the actual argument to the expected type. If it isn't, then passing a long long argument to a variadic function, where a long is expected, results in Undefined Behavior. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: AW: [PATCH 5/5] Bionic lacks transparent LFS migrations; provide a workaround
On Tuesday, 28 April 2015, at 6:36 am, dietmar.schind...@manroland-web.com wrote: > Whose policy is that which caters to a certain compiler option's inept > warnings? "What are the goals of Busybox?" > (http://www.busybox.net/FAQ.html#goals) says: "We also want to have the > simplest and cleanest implementation we can manage..." - I wouldn't say that > > #if defined(__BIONIC__) && defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == > 64 > > is as simple and clean as > > #if defined(__BIONIC__) && _FILE_OFFSET_BITS == 64 I originally had it as you suggest, without the redundant check, but I saw the compiler warning and added the extra check to avoid the warning. Frankly, I consider having no warnings to be "cleaner," even if I have to add a little bit of visual noise to the code. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Bionic lacks transparent LFS migrations; provide a workaround
On Tuesday, 28 April 2015, at 9:56 am, Rich Felker wrote: > What I do think is a bad idea is providing a hackish environment where > only some things work correctly with 64-bit off_t and others silently > fail or even misinterpret their arguments and blow up. If that can be > avoided and the ugliness can be contained in a pit that's isolated to > Bionic, I'm not opposed to it. For what it's worth, BusyBox *presently* misinterprets arguments on Bionic because Bionic's struct stat has 64-bit st_ino and st_size, but BusyBox happily assigns the values of these fields into variables of type ino_t and off_t, which breaks at run-time. These problems would be apparent at compile-time too, except that BusyBox's build system doesn't specify -Wconversion, so no warnings are produced. Specifying -Wconversion reveals numerous places throughout the BusyBox sources where rvalues are implicitly narrowed. My personal belief is that these warnings should always be enabled, and implicit narrowing conversions should always be avoided by using explicit casts, to make obvious that a change in value is possible, but I am not here to convince the BusyBox devs to eliminate all the implicit narrowing conversions in BusyBox. Besides, I agree that Bionic is at fault for deviating from the standard here by declaring st_ino and st_size as unsigned long long and long long rather than as ino_t and off_t. I'll work on finishing the Large File Support in Bionic and adding support for _FILE_OFFSET_BITS==64. Is there any chance that a Bionic headers patch could be shipped with BusyBox? Again, there is no reason to require BusyBox users to compile against the latest Bionic headers just to get support for large files. In fact, LFS could be implemented on all versions of Bionic with a simple "-include" in CFLAGS. (That's almost no different than implementing it in platform.h, but maybe you'd like it better in a separate file.) ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Bionic lacks transparent LFS migrations; provide a workaround
On Sunday, 26 April 2015, at 10:28 pm, Rich Felker wrote: > On Sat, Apr 25, 2015 at 04:06:23PM -0400, Matt Whitlock wrote: > > In short, Bionic has most of the syscall-level support introduced by > > _LARGEFILE64_SOURCE in Glibc, but they did not implement the > > transparent migration introduced by _FILE_OFFSET_BITS==64. This is > > easy to implement atop Bionic, however, as it is simply a matter of > > mapping the relevant 32-bit types and functions to their 64-bit > > equivalents. (See feature_test_macros(7).) > > This really can and should be fixed at the header level for Bionic. > Nobody should be writing foo64 in sources except as a transparent > workaround for broken systems. While you could do the workaround in > Busybox, I think it would be much more useful as a patch to the Bionic > headers that anyone can apply to the installed headers to fix them, > and a patch to send upstream to get the source of the problem fixed. The Bionic headers aren't broken. _FILE_OFFSET_BITS is a convenience provided by Glibc, but it is not specified by any standard. If you think it's a bad idea to map off_t to off64_t within BusyBox, then another option would be to define a bb_off_t type that is equivalent to off_t when compiling without _LARGEFILE64_SOURCE or else equivalent to off64_t when compiling with _LARGEFILE64_SOURCE. There would be similar definitions of bb_lseek(), bb_pread(), bb_pwrite(), bb_ftruncate(), and other LFS functions in the future, as Bionic implements them. The changeset necessary to switch BusyBox to using this new type and these new functions would be rather expansive, and (in my opinion) it seems undesirable to make such an unintuitive change just to support one flavor of libc. I will submit a patch to the Bionic devs to add support for _FILE_OFFSET_BITS, but frankly I don't expect much cooperation by them, and even if they do apply the patch, it would only affect new versions of Bionic. There is no reason BusyBox can't have large file support on older versions of Bionic as well. Passing the buck upstream consigns support for large files in BusyBox to the future when BusyBox could easily have such support now. > > Bionic does *not* implement the 64-bit stdio functions, but those > > use a different type (fpos_t instead of off_t) and so are not > > affected by this patch. > > That's something of a mischaracterization. The only useful stdio > functions use off_t: fseeko and ftello. fgetpos and fsetpos are rather > useless since all they can do is save/restore a position you've > already reached as an abstract position object. They cannot work with > any kind of sequential addresses. Point taken. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Bionic lacks transparent LFS migrations; provide a workaround
On Sunday, 26 April 2015, at 12:40 pm, Tanguy Pruvot wrote: > On which bionic version are you doing the tests ? 64bit arm/x86 devices are > recent. I'm building using a toolchain for Android API level 15 and testing on an ARM device running KitKat. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Bionic lacks transparent LFS migrations; provide a workaround
On Saturday, 25 April 2015, at 7:17 pm, Denys Vlasenko wrote: > Let me know how I can test this on a x86 machine. Bionic doesn't really support vanilla Linux. The Android Developers' Bionic C Library Overview says: «Note that the x86 version is only meant to run on an x86 Android device. We make absolutely no claim that you could build and use Bionic on a stock x86 Linux distribution (though that would be cool, so patches are welcomed :-))» If you have an x86-based Android device, then it should simply be a matter of compiling BusyBox using an x86-linux-androideabi toolchain. I've only tested using an arm-linux-androideabi toolchain, so I can't really help you there. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Bionic lacks transparent LFS migrations; provide a workaround
On Saturday, 25 April 2015, at 11:38 am, Rich Felker wrote: > On Sat, Apr 25, 2015 at 04:53:33AM -0400, Matt Whitlock wrote: > > This solves some of the problems arising from Bionic's off_t being > > 32 bits wide despite _FILE_OFFSET_BITS==64. See BusyBox bug #6908. > > > > Note that this doesn't solve all such problems since Bionic lacks > > 64-bit variants of many standard I/O functions: open, creat, lockf, > > posix_fadvise, posix_fallocate, truncate, sendfile, getrlimit, > > setrlimit, fopen, freopen, fseeko, ftello, fgetpos, fsetpos, > > mkstemp. > > As long as that's the case, I think this approach of trying to fake > 64-bit off_t on Bionic is dangerous and probably best omitted. IMO we > should either be pushing for Bionic to fix these things or sticking > with 32-bit off_t on Bionic and treating that as a limitation of the > platform. Don't throw the baby out with the bathwater. Bionic *does* support large files. In fact, it's mandatory in some cases: there are no 32-bit versions of stat(), lstat(), fstat(), fstatat(), statfs(), fstatfs(), or getdents(); and struct stat, struct statfs, and struct dirent are equivalent to struct stat64, struct statfs64, and struct dirent64 from Glibc, respectively. Furthermore, while Bionic does support 32-bit lseek(), pread(), pwrite(), and ftruncate() with 32-bit off_t, it does also have lseek64(), pread64(), pwrite64(), and ftruncate64() with off64_t. In short, Bionic has most of the syscall-level support introduced by _LARGEFILE64_SOURCE in Glibc, but they did not implement the transparent migration introduced by _FILE_OFFSET_BITS==64. This is easy to implement atop Bionic, however, as it is simply a matter of mapping the relevant 32-bit types and functions to their 64-bit equivalents. (See feature_test_macros(7).) Bionic does *not* implement the 64-bit stdio functions, but those use a different type (fpos_t instead of off_t) and so are not affected by this patch. Would you be more willing to apply this patch if I were to produce a unit test suite demonstrating that 64-bit file syscalls work with off_t while stdio functions remain 32-bit with fpos_t? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] Bionic lacks ttyname_r; provide a workaround
---
include/platform.h | 7 +++
libbb/platform.c | 17 +
2 files changed, 24 insertions(+)
diff --git a/include/platform.h b/include/platform.h
index 8914d4a..8896a6b 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -368,6 +368,7 @@ typedef unsigned smalluint;
#define HAVE_DPRINTF 1
#define HAVE_MEMRCHR 1
#define HAVE_MKDTEMP 1
+#define HAVE_TTYNAME_R 1
#define HAVE_PTSNAME_R 1
#define HAVE_SETBIT 1
#define HAVE_SIGHANDLER_T 1
@@ -480,6 +481,7 @@ typedef unsigned smalluint;
#if defined(ANDROID) || defined(__ANDROID__)
# undef HAVE_DPRINTF
+# undef HAVE_TTYNAME_R
# undef HAVE_GETLINE
# undef HAVE_STPCPY
# undef HAVE_MEMPCPY
@@ -506,6 +508,11 @@ extern void *memrchr(const void *s, int c, size_t n)
FAST_FUNC;
extern char *mkdtemp(char *template) FAST_FUNC;
#endif
+#ifndef HAVE_TTYNAME_R
+#define ttyname_r bb_ttyname_r
+extern int ttyname_r(int fd, char *buf, size_t buflen);
+#endif
+
#ifndef HAVE_SETBIT
# define setbit(a, b) ((a)[(b) >> 3] |= 1 << ((b) & 7))
# define clrbit(a, b) ((a)[(b) >> 3] &= ~(1 << ((b) & 7)))
diff --git a/libbb/platform.c b/libbb/platform.c
index 8d90ca4..c676f17 100644
--- a/libbb/platform.c
+++ b/libbb/platform.c
@@ -113,6 +113,23 @@ char* FAST_FUNC mkdtemp(char *template)
}
#endif
+#ifndef HAVE_TTYNAME_R
+int ttyname_r(int fd, char *buf, size_t buflen)
+{
+ int r;
+ char path[32];
+ if (!isatty(fd))
+ return errno == EINVAL ? ENOTTY : errno;
+ sprintf(path, "/proc/self/fd/%d", fd);
+ if ((r = readlink(path, buf, buflen)) < 0)
+ return errno;
+ if (r >= buflen)
+ return ERANGE;
+ buf[r] = '\0';
+ return 0;
+}
+#endif
+
#ifndef HAVE_STRCASESTR
/* Copyright (c) 1999, 2000 The ht://Dig Group */
char* FAST_FUNC strcasestr(const char *s, const char *pattern)
--
2.3.5
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] Bionic lacks transparent LFS migrations; provide a workaround
This solves some of the problems arising from Bionic's off_t being 32 bits wide despite _FILE_OFFSET_BITS==64. See BusyBox bug #6908. Note that this doesn't solve all such problems since Bionic lacks 64-bit variants of many standard I/O functions: open, creat, lockf, posix_fadvise, posix_fallocate, truncate, sendfile, getrlimit, setrlimit, fopen, freopen, fseeko, ftello, fgetpos, fsetpos, mkstemp. --- include/platform.h | 21 + 1 file changed, 21 insertions(+) diff --git a/include/platform.h b/include/platform.h index 8896a6b..56dc02b 100644 --- a/include/platform.h +++ b/include/platform.h @@ -491,6 +491,27 @@ typedef unsigned smalluint; # undef HAVE_NET_ETHERNET_H #endif +/* Bionic system headers lack transparent LFS migrations. */ +#if defined(__BIONIC__) && defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64 +/* Must include before migrating off_t to off64_t. */ +#include +/* from */ +# define lseek lseek64 +# define pread pread64 +# define pwrite pwrite64 +# define ftruncate ftruncate64 +/* from */ +# define off_t off64_t +# define ino_t ino64_t +# define blkcnt_t blkcnt64_t +# define fsblkcnt_t fsblkcnt64_t +# define fsfilcnt_t fsfilcnt64_t +typedef uint64_t ino_t; +typedef uint64_t blkcnt_t; +typedef uint64_t fsblkcnt_t; +typedef uint64_t fsfilcnt_t; +#endif + /* * Now, define prototypes for all the functions defined in platform.c * These must come after all the HAVE_* macros are defined (or not) -- 2.3.5 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] Bionic lacks transparent LFS migrations; provide a workaround
On Friday, 24 April 2015, at 5:42 pm, Matt Whitlock wrote: > This solves some of the problems arising from Bionic's off_t being 32 bits > wide despite _FILE_OFFSET_BITS==64. See BusyBox bug #6908. I found an issue with this patch: certain stdio functions were segfaulting. It was because was being included after off_t had already been redefined to off64_t, and this was causing some function prototypes and structure definitions to diverge from their counterparts in Bionic's libc. I have a revision of the patch forthcoming to address this issue. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] Bionic lacks setmntent, getmntent_r, endmntent; provide workarounds
Note: This commit subsumes a previous workaround (908d6b70, a0e17f7d) for the
lack of getmntent_r in dietlibc.
---
include/platform.h | 39 +++
libbb/platform.c| 47 +++
util-linux/mount.c | 12
util-linux/umount.c | 12
4 files changed, 86 insertions(+), 24 deletions(-)
diff --git a/include/platform.h b/include/platform.h
index df95945..aa7fddd 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -384,6 +384,11 @@ typedef unsigned smalluint;
#define HAVE_UNLOCKED_LINE_OPS 1
#define HAVE_GETLINE 1
#define HAVE_XTABS 1
+#define HAVE_SETMNTENT 1
+#define HAVE_GETMNTENT 1
+#define HAVE_GETMNTENT_R 1
+#define HAVE_ADDMNTENT 1
+#define HAVE_ENDMNTENT 1
#define HAVE_MNTENT_H 1
#define HAVE_NET_ETHERNET_H 1
#define HAVE_SYS_STATFS_H 1
@@ -446,6 +451,7 @@ typedef unsigned smalluint;
#if defined(__dietlibc__)
# undef HAVE_STRCHRNUL
+# undef HAVE_GETMNTENT_R
#endif
#if defined(__APPLE__)
@@ -485,6 +491,11 @@ typedef unsigned smalluint;
# undef HAVE_STRCHRNUL
# undef HAVE_STRVERSCMP
# undef HAVE_UNLOCKED_LINE_OPS
+# undef HAVE_SETMNTENT
+# undef HAVE_GETMNTENT
+# undef HAVE_GETMNTENT_R
+# undef HAVE_ADDMNTENT
+# undef HAVE_ENDMNTENT
# undef HAVE_NET_ETHERNET_H
#endif
@@ -561,4 +572,32 @@ extern int vasprintf(char **string_ptr, const char
*format, va_list p) FAST_FUNC
extern ssize_t getline(char **lineptr, size_t *n, FILE *stream) FAST_FUNC;
#endif
+#ifndef HAVE_SETMNTENT
+# include /* for FILE */
+# define setmntent bb_setmntent
+extern FILE * setmntent(const char *filename, const char *type);
+#endif
+
+#ifndef HAVE_GETMNTENT
+# include /* for FILE */
+extern struct mntent * getmntent(FILE *stream);
+#endif
+
+#ifndef HAVE_GETMNTENT_R
+# include /* for FILE */
+# define getmntent_r bb_getmntent_r
+extern struct mntent * getmntent_r(FILE *streamp, struct mntent *mntbuf, char
*buf, int buflen);
+#endif
+
+#ifndef HAVE_ADDMNTENT
+# include /* for FILE */
+extern int addmntent(FILE *stream, const struct mntent *mnt);
+#endif
+
+#ifndef HAVE_ENDMNTENT
+# include /* for FILE */
+# define endmntent bb_endmntent
+extern int endmntent(FILE *streamp);
+#endif
+
#endif
diff --git a/libbb/platform.c b/libbb/platform.c
index 8d90ca4..c6c2526 100644
--- a/libbb/platform.c
+++ b/libbb/platform.c
@@ -194,3 +194,50 @@ ssize_t FAST_FUNC getline(char **lineptr, size_t *n, FILE
*stream)
return len;
}
#endif
+
+#ifndef HAVE_SETMNTENT
+FILE *setmntent(const char *filename, const char *type)
+{
+ return fopen(filename, type);
+}
+#endif
+
+#ifndef HAVE_GETMNTENT_R
+# ifdef HAVE_GETMNTENT
+struct mntent * getmntent_r(FILE *streamp, struct mntent *mntbuf,
+ char *buf UNUSED_PARAM, int buflen UNUSED_PARAM)
+{
+ struct mntent *ent = getmntent(streamp);
+ return ent ? memcpy(mntbuf, ent, sizeof *mntbuf) : NULL;
+}
+# else
+struct mntent * getmntent_r(FILE *streamp, struct mntent *mntbuf, char *buf,
int buflen)
+{
+ static const char delims[] = " \t";
+ char *saveptr;
+ while (fgets(buf, buflen, streamp)) {
+ if ((mntbuf->mnt_fsname = strtok_r(buf, delims, &saveptr)) &&
+ mntbuf->mnt_fsname[0] != '#' &&
+ (mntbuf->mnt_dir = strtok_r(NULL, delims,
&saveptr)) &&
+ (mntbuf->mnt_type = strtok_r(NULL, delims,
&saveptr)) &&
+ (mntbuf->mnt_opts = strtok_r(NULL, delims,
&saveptr))) {
+ mntbuf->mnt_passno = mntbuf->mnt_freq = 0;
+ if ((buf = strtok_r(NULL, delims, &saveptr))) {
+ mntbuf->mnt_freq = atoi(buf);
+ if ((buf = strtok_r(NULL, delims, &saveptr)))
+ mntbuf->mnt_passno = atoi(buf);
+ }
+ return mntbuf;
+ }
+ }
+ return NULL;
+}
+# endif
+#endif
+
+#ifndef HAVE_ENDMNTENT
+int endmntent(FILE *streamp)
+{
+ return fclose(streamp);
+}
+#endif
diff --git a/util-linux/mount.c b/util-linux/mount.c
index cb40c80..3d179a7 100644
--- a/util-linux/mount.c
+++ b/util-linux/mount.c
@@ -245,18 +245,6 @@
#endif
-#if defined(__dietlibc__)
-// 16.12.2006, Sampo Kellomaki (sa...@iki.fi)
-// dietlibc-0.30 does not have implementation of getmntent_r()
-static struct mntent *getmntent_r(FILE* stream, struct mntent* result,
- char* buffer UNUSED_PARAM, int bufsize UNUSED_PARAM)
-{
- struct mntent* ment = getmntent(stream);
- return memcpy(result, ment, sizeof(*ment));
-}
-#endif
-
-
// Not real flags, but we want to be able to check for this.
enum {
MOUNT_USERS = (1 << 28) * ENABLE_DESKTOP,
diff --git a/util-linux/umount.c b/util-linux/umount.c
index 4c2e882..2afdf59 100644
--- a/util-linux/umount.c
+++ b/util-linux/umount.c
@@ -32,18 +32,6 @@
#include
#include
[PATCH 1/2] Bionic lacks ttyname_r; provide a workaround
---
include/platform.h | 7 +++
libbb/missing_syscalls.c | 17 +
2 files changed, 24 insertions(+)
diff --git a/include/platform.h b/include/platform.h
index 8914d4a..8896a6b 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -368,6 +368,7 @@ typedef unsigned smalluint;
#define HAVE_DPRINTF 1
#define HAVE_MEMRCHR 1
#define HAVE_MKDTEMP 1
+#define HAVE_TTYNAME_R 1
#define HAVE_PTSNAME_R 1
#define HAVE_SETBIT 1
#define HAVE_SIGHANDLER_T 1
@@ -480,6 +481,7 @@ typedef unsigned smalluint;
#if defined(ANDROID) || defined(__ANDROID__)
# undef HAVE_DPRINTF
+# undef HAVE_TTYNAME_R
# undef HAVE_GETLINE
# undef HAVE_STPCPY
# undef HAVE_MEMPCPY
@@ -506,6 +508,11 @@ extern void *memrchr(const void *s, int c, size_t n)
FAST_FUNC;
extern char *mkdtemp(char *template) FAST_FUNC;
#endif
+#ifndef HAVE_TTYNAME_R
+#define ttyname_r bb_ttyname_r
+extern int ttyname_r(int fd, char *buf, size_t buflen);
+#endif
+
#ifndef HAVE_SETBIT
# define setbit(a, b) ((a)[(b) >> 3] |= 1 << ((b) & 7))
# define clrbit(a, b) ((a)[(b) >> 3] &= ~(1 << ((b) & 7)))
diff --git a/libbb/missing_syscalls.c b/libbb/missing_syscalls.c
index dd430e3..c768f11 100644
--- a/libbb/missing_syscalls.c
+++ b/libbb/missing_syscalls.c
@@ -40,3 +40,20 @@ int pivot_root(const char *new_root, const char *put_old)
return syscall(__NR_pivot_root, new_root, put_old);
}
#endif
+
+#ifndef HAVE_TTYNAME_R
+int ttyname_r(int fd, char *buf, size_t buflen)
+{
+ int r;
+ char path[32];
+ if (!isatty(fd))
+ return errno == EINVAL ? ENOTTY : errno;
+ sprintf(path, "/proc/self/fd/%d", fd);
+ if ((r = readlink(path, buf, buflen)) < 0)
+ return errno;
+ if (r >= buflen)
+ return ERANGE;
+ buf[r] = '\0';
+ return 0;
+}
+#endif
--
2.0.5
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 2/2] Bionic lacks tcdrain; provide a workaround
---
libbb/missing_syscalls.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/libbb/missing_syscalls.c b/libbb/missing_syscalls.c
index c768f11..1e2507d 100644
--- a/libbb/missing_syscalls.c
+++ b/libbb/missing_syscalls.c
@@ -39,6 +39,11 @@ int pivot_root(const char *new_root, const char *put_old)
{
return syscall(__NR_pivot_root, new_root, put_old);
}
+
+int tcdrain(int fd)
+{
+ return ioctl(fd, TCSBRK, 1);
+}
#endif
#ifndef HAVE_TTYNAME_R
--
2.0.5
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] Bionic lacks transparent LFS migrations; provide a workaround
This solves some of the problems arising from Bionic's off_t being 32 bits wide despite _FILE_OFFSET_BITS==64. See BusyBox bug #6908. Note that this doesn't solve all such problems since Bionic lacks 64-bit variants of many standard I/O functions: open, creat, lockf, posix_fadvise, posix_fallocate, truncate, sendfile, getrlimit, setrlimit, fopen, freopen, fseeko, ftello, fgetpos, fsetpos, mkstemp. --- include/platform.h | 19 +++ 1 file changed, 19 insertions(+) diff --git a/include/platform.h b/include/platform.h index 9893536..1744b3c 100644 --- a/include/platform.h +++ b/include/platform.h @@ -491,6 +491,25 @@ typedef unsigned smalluint; # undef HAVE_NET_ETHERNET_H #endif +/* Bionic system headers lack transparent LFS migrations. */ +#if defined(__BIONIC__) && defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64 +/* from */ +# define lseek lseek64 +# define pread pread64 +# define pwrite pwrite64 +# define ftruncate ftruncate64 +/* from */ +# define off_t off64_t +# define ino_t ino64_t +# define blkcnt_t blkcnt64_t +# define fsblkcnt_t fsblkcnt64_t +# define fsfilcnt_t fsfilcnt64_t +typedef uint64_t ino_t; +typedef uint64_t blkcnt_t; +typedef uint64_t fsblkcnt_t; +typedef uint64_t fsfilcnt_t; +#endif + /* * Now, define prototypes for all the functions defined in platform.c * These must come after all the HAVE_* macros are defined (or not) -- 2.0.5 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 5/5] Bionic lacks transparent LFS migrations; provide a workaround
On Friday, 24 April 2015, at 4:12 am, Matt Whitlock wrote: > +#if defined(__BIONIC__) && _FILE_OFFSET_BITS == 64 The preprocessor needs to test whether _FILE_OFFSET_BITS is defined before using it in an expression. #if defined(__BIONIC__) && defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS == 64 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 1/2] Bionic lacks ttyname_r; provide a workaround
I think providing an alternative implementation of ttyname_r() in missing_syscalls.c won't work. Busybox cannot be statically linked when ttyname_r is defined in missing_syscalls.o: ld: error: /usr/local/arm-linux-androideabi/bin/../sysroot/usr/lib/libc.a(stubs.o): multiple definition of 'ttyname_r' ld: libbb/lib.a(missing_syscalls.o): previous definition here ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 1/2] Bionic lacks ttyname_r; provide a workaround
---
libbb/missing_syscalls.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/libbb/missing_syscalls.c b/libbb/missing_syscalls.c
index dd430e3..e57c2de 100644
--- a/libbb/missing_syscalls.c
+++ b/libbb/missing_syscalls.c
@@ -39,4 +39,18 @@ int pivot_root(const char *new_root, const char *put_old)
{
return syscall(__NR_pivot_root, new_root, put_old);
}
+
+int ttyname_r(int fd, char *buf, size_t buflen) {
+ int r;
+ char path[32];
+ if (!isatty(fd))
+ return errno == EINVAL ? ENOTTY : errno;
+ sprintf(path, "/proc/self/fd/%d", fd);
+ if ((r = readlink(path, buf, buflen)) < 0)
+ return errno;
+ if (r >= buflen)
+ return ERANGE;
+ buf[r] = '\0';
+ return 0;
+}
#endif
--
2.0.5
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 2/2] Bionic lacks tcdrain; provide a workaround
---
libbb/missing_syscalls.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/libbb/missing_syscalls.c b/libbb/missing_syscalls.c
index e57c2de..0de3df9 100644
--- a/libbb/missing_syscalls.c
+++ b/libbb/missing_syscalls.c
@@ -40,6 +40,11 @@ int pivot_root(const char *new_root, const char *put_old)
return syscall(__NR_pivot_root, new_root, put_old);
}
+int tcdrain(int fd)
+{
+ return ioctl(fd, TCSBRK, 1);
+}
+
int ttyname_r(int fd, char *buf, size_t buflen) {
int r;
char path[32];
--
2.0.5
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 3/5] Bionic lacks ttyname_r; provide a workaround
On Friday, 24 April 2015, at 1:04 pm, walter harms wrote: > perhaps it is better to move the code to missing_syscalls.c ? > (this is a question) The reason I didn't put it there is that ttyname_r() is defined in Bionic, but it's a stub that prints out a diagnostic message at runtime and returns -1 unconditionally, thereby causing xmalloc_ttyname() to fail, even when the specified file descriptor does refer to a TTY. I'll provide an alternative patch that places the workaround in missing_syscalls.c since I can see good arguments for both ways. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 4/5] Bionic lacks tcdrain; provide a workaround
---
libbb/missing_syscalls.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/libbb/missing_syscalls.c b/libbb/missing_syscalls.c
index dd430e3..e3c1e92 100644
--- a/libbb/missing_syscalls.c
+++ b/libbb/missing_syscalls.c
@@ -39,4 +39,9 @@ int pivot_root(const char *new_root, const char *put_old)
{
return syscall(__NR_pivot_root, new_root, put_old);
}
+
+int tcdrain(int fd)
+{
+ return ioctl(fd, TCSBRK, 1);
+}
#endif
--
2.0.5
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 1/5] bb_socklen_t must be signed int
BSD and POSIX specify socklen_t as int. Busybox should too, to avoid pointer signedness warnings. --- include/libbb.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/libbb.h b/include/libbb.h index f0ac1f5..dc0acd8 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -143,7 +143,7 @@ * (in case "is it defined already" detection above failed) */ # define socklen_t bb_socklen_t - typedef unsigned socklen_t; + typedef int socklen_t; # endif #endif #ifndef HAVE_CLEARENV -- 2.0.5 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 2/5] Bionic lacks mempcpy; enable existing workaround
--- include/platform.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/platform.h b/include/platform.h index df95945..8914d4a 100644 --- a/include/platform.h +++ b/include/platform.h @@ -482,6 +482,7 @@ typedef unsigned smalluint; # undef HAVE_DPRINTF # undef HAVE_GETLINE # undef HAVE_STPCPY +# undef HAVE_MEMPCPY # undef HAVE_STRCHRNUL # undef HAVE_STRVERSCMP # undef HAVE_UNLOCKED_LINE_OPS -- 2.0.5 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 3/5] Bionic lacks ttyname_r; provide a workaround
---
include/platform.h| 2 ++
libbb/xfuncs_printf.c | 10 ++
2 files changed, 12 insertions(+)
diff --git a/include/platform.h b/include/platform.h
index 8914d4a..9893536 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -369,6 +369,7 @@ typedef unsigned smalluint;
#define HAVE_MEMRCHR 1
#define HAVE_MKDTEMP 1
#define HAVE_PTSNAME_R 1
+#define HAVE_TTYNAME_R 1
#define HAVE_SETBIT 1
#define HAVE_SIGHANDLER_T 1
#define HAVE_STPCPY 1
@@ -481,6 +482,7 @@ typedef unsigned smalluint;
#if defined(ANDROID) || defined(__ANDROID__)
# undef HAVE_DPRINTF
# undef HAVE_GETLINE
+# undef HAVE_TTYNAME_R
# undef HAVE_STPCPY
# undef HAVE_MEMPCPY
# undef HAVE_STRCHRNUL
diff --git a/libbb/xfuncs_printf.c b/libbb/xfuncs_printf.c
index e4ac6a0..bada57f 100644
--- a/libbb/xfuncs_printf.c
+++ b/libbb/xfuncs_printf.c
@@ -569,9 +569,19 @@ int FAST_FUNC bb_xioctl(int fd, unsigned request, void
*argp)
char* FAST_FUNC xmalloc_ttyname(int fd)
{
char buf[128];
+#ifdef HAVE_TTYNAME_R
int r = ttyname_r(fd, buf, sizeof(buf) - 1);
if (r)
return NULL;
+#else
+ int r;
+ if (!isatty(fd))
+ return NULL;
+ sprintf(buf, "/proc/self/fd/%d", fd);
+ if ((r = readlink(buf, buf, sizeof(buf) - 1)) < 0)
+ return NULL;
+ buf[r] = '\0';
+#endif
return xstrdup(buf);
}
--
2.0.5
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 5/5] Bionic lacks transparent LFS migrations; provide a workaround
This solves some of the problems arising from Bionic's off_t being 32 bits wide despite _FILE_OFFSET_BITS==64. See BusyBox bug #6908. Note that this doesn't solve all such problems since Bionic lacks 64-bit variants of many standard I/O functions: open, creat, lockf, posix_fadvise, posix_fallocate, truncate, sendfile, getrlimit, setrlimit, fopen, freopen, fseeko, ftello, fgetpos, fsetpos, mkstemp. --- include/platform.h | 19 +++ 1 file changed, 19 insertions(+) diff --git a/include/platform.h b/include/platform.h index 9893536..4540e0a 100644 --- a/include/platform.h +++ b/include/platform.h @@ -491,6 +491,25 @@ typedef unsigned smalluint; # undef HAVE_NET_ETHERNET_H #endif +/* Bionic system headers lack transparent LFS migrations. */ +#if defined(__BIONIC__) && _FILE_OFFSET_BITS == 64 +/* from */ +# define lseek lseek64 +# define pread pread64 +# define pwrite pwrite64 +# define ftruncate ftruncate64 +/* from */ +# define off_t off64_t +# define ino_t ino64_t +# define blkcnt_t blkcnt64_t +# define fsblkcnt_t fsblkcnt64_t +# define fsfilcnt_t fsfilcnt64_t +typedef uint64_t ino_t; +typedef uint64_t blkcnt_t; +typedef uint64_t fsblkcnt_t; +typedef uint64_t fsfilcnt_t; +#endif + /* * Now, define prototypes for all the functions defined in platform.c * These must come after all the HAVE_* macros are defined (or not) -- 2.0.5 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: Anyone using udhcpc6
On 6-May-2014, at 3:19 pm, "Harshad Prabhu (hprabhu)" wrote: > Let me know if anyone has added (any patches) / using udhcpc6 support in > busybox. I *would* be using udhcpc6 if it were working. I've tried, and I just can't make it recognize the DHCPv6 response from Comcast's CMTS. I'm using Dibbler as an alternative right now, but as soon as udhcpc6 supports obtaining an IPv6 Prefix Delegation from a DHCPv6 server, I'd love to switch to it instead and eliminate the massive beast that is Dibbler. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: udhcpc6 and Prefix Delegation
On Friday, 28 March 2014, at 3:54 pm, Julien Laffaye wrote: > Well, with udhcpc6, it keeps sending discovers and never get a response. > wide-dhcpv6 works on the same link. > > (I should have started by specifying that in my first email) > > I guess I will capture the traffic with both dhcp clients and see what > differs. I likewise tried udhcpc6 and couldn't get it to work. It mentions that it's receiving packets but that it's considering them "unrelated/bogus" and so ignoring them. I went with Dibbler and am now happily distributing my DHCPv6-assigned prefix delegation to my LAN via SLAAC. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: /proc//cmdline and udhcpcd
On Wednesday, 26 March 2014, at 1:38 pm, Cristian Ionescu-Idbohrn wrote: > On Wed, 26 Mar 2014, Matt Whitlock wrote: > > Tons of programs modify their argv arrays. > > Alright, you know better. It's common for programs that accept sensitive information (such as passwords) via command-line arguments to null out those arguments so they can't be read via /proc//cmdline or 'ps'. > > Anything that uses strtok to split arguments at delimiters will do > > it. > > Yes, there's still much to learn. Do you happen to know of some > popular examples? Mplayer comes to mind immediately, as the suboptions for its filters are typically delimited with colons on the command line, but those suboptions then get parsed apart. > > One should never assume that /proc//cmdline will contain > > the command line as it was originally used to execute the process. > > Do you happen to know of some reliable reference text stating that? I've been looking around a little, but all the sources I've found explaining what /proc//cmdline is just say it gives the arguments used to start the process. While it's true that cmdline does return the bytes at the location of the arguments that were used to start the process, it's not guaranteed that those bytes are unaltered since the process was started. That's the reason one should never assume. The argv parameter to the main function in a C program is passed as a pointer to an array of pointers to *non-constant* characters, meaning the program is given explicit permission to modify those characters however it likes. The kernel does not make a backup of the contents of the argument array passed to execve, so cmdline reads from the only copy of those arguments that is available, which is the copy that has been passed to the executed process, which is free to change those bytes. It's worth mentioning that this is exactly the same behavior as /proc//environ, whic h contains the environment of the process, which the process is free to change at will as well, and which changes will be reflected in /proc//environ. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: /proc//cmdline and udhcpcd
On Wednesday, 26 March 2014, at 1:19 pm, Cristian Ionescu-Idbohrn wrote: > But really, this is not an `init' question, as I see it. It's about > keeping the original/unmodified program arguments in > /proc//cmdline. > > I know of no other program doing such sort of thing, although I think > I understand the reason behind this particular way of handling the > '-x' option argument in udhcpc. Tons of programs modify their argv arrays. Anything that uses strtok to split arguments at delimiters will do it. One should never assume that /proc//cmdline will contain the command line as it was originally used to execute the process. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: /proc//cmdline and udhcpcd
On Wednesday, 26 March 2014, at 12:46 pm, Cristian Ionescu-Idbohrn wrote: > Is it possible to restrain from modifying the arguments for the > command line? The problem is that the process watcher used here is > dependant on having the original/unmodified /proc//cmdline. If > the watched process dies, the cmdline is used to respawn it. > Respawning it with an erroneous command line creates a respawn loop. Couldn't you use the 'init' applet from Busybox to achieve process watching and respawning? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: /proc//cmdline and udhcpcd
On Tuesday, 25 March 2014, at 10:52 pm, Cristian Ionescu-Idbohrn wrote: > I find out that using that option: > > -x hostname:foo > ^ > > shows up in /proc//cmdline as: > > -x hostname foo > ^ /proc//cmdline reflects any changes that the process has made to its argv array. It's common when parsing a command line for a program to replace delimiters with null bytes. The /proc//cmdline interface converts null bytes to spaces for ease of display and parsing. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] add discard option -d to swapon
On Monday, 24 March 2014, at 12:06 am, Tito wrote:
> On Sunday 23 March 2014 23:09:59 you wrote:
> > This is still going to create a "hell" when adding the discard flag.
> > Consider what will happen in the enum: if DISCARD is enabled but PRI is
> > disabled, then OPT_d will be (1 << 1), but if both are enabled, then OPT_p
> > will be (1 << 1) and OPT_d will be (1 << 2) (or vice versa). Imagine adding
> > five more optional features. It leads to a combinatorial explosion. It's
> > cleaner to downshift the bitmap after checking each bit within its
> > appropriate #if section.
>
> The problem with flags could be solved easily, take a look at how it is done
> in df.c:
>
> enum {
> OPT_KILO = (1 << 0),
> OPT_POSIX = (1 << 1),
> OPT_ALL = (1 << 2) * ENABLE_FEATURE_DF_FANCY,
> OPT_INODE = (1 << 3) * ENABLE_FEATURE_DF_FANCY,
> OPT_BSIZE = (1 << 4) * ENABLE_FEATURE_DF_FANCY,
> OPT_HUMAN = (1 << (2 + 3*ENABLE_FEATURE_DF_FANCY)) *
> ENABLE_FEATURE_HUMAN_READABLE,
> OPT_MEGA = (1 << (3 + 3*ENABLE_FEATURE_DF_FANCY)) *
> ENABLE_FEATURE_HUMAN_READABLE,
> };
>
> that way the enum is independent from the combination of options enabled and
> most #ifdefs could be removed
> and we are lucky becuase we have only 3 flags. ;-)
That would start to get quite messy as the number of options increases:
enum {
OPT_a = (1 << 0), /* all */
OPT_d = (1 << 1) * ENABLE_FEATURE_SWAPON_DISCARD,
OPT_f = (1 << (1 + ENABLE_FEATURE_SWAPON_DISCARD)) *
ENABLE_FEATURE_SWAPON_FIXPGSZ,
OPT_p = (1 << (1 + ENABLE_FEATURE_SWAPON_DISCARD +
ENABLE_FEATURE_SWAPON_FIXPGSZ)) * ENABLE_FEATURE_SWAPON_PRI,
OPT_s = (1 << (1 + ENABLE_FEATURE_SWAPON_DISCARD +
ENABLE_FEATURE_SWAPON_FIXPGSZ + ENABLE_FEATURE_SWAPON_PRI)) *
ENABLE_FEATURE_SWAPON_SUMMARY
};
Bleh.
I prefer this approach:
ret = getopt32(argv,
IF_FEATURE_SWAPON_DISCARD("d::")
IF_FEATURE_SWAPON_FIXPGSZ("f")
IF_FEATURE_SWAPON_PRI("p:")
IF_FEATURE_SWAPON_SUMMARY("s")
"a"
IF_FEATURE_SWAPON_DISCARD(, &discard)
IF_FEATURE_SWAPON_PRI(, &prio)
);
#if ENABLE_FEATURE_SWAPON_DISCARD
if (ret & 1) { // -d
/* ...handle discard option... */
}
ret >>= 1;
#endif
#if ENABLE_FEATURE_SWAPON_FIXPGSZ
if (ret & 1) { // -f
/* ...handle fixpgsz option... */
}
ret >>= 1;
#endif
#if ENABLE_FEATURE_SWAPON_PRI
if (ret & 1) { // -p
/* ...handle priority option... */
}
ret >>= 1;
#endif
#if ENABLE_FEATURE_SWAPON_SUMMARY
if (ret & 1) { // -s
/* ...handle summary option... */
}
ret >>= 1;
#endif
if (ret /* & 1: redundant */) { // -a
/* ...handle all option... */
}
That's much cleaner and more scalable.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] add discard option -d to swapon
On Sunday, 23 March 2014, at 8:29 pm, Tito wrote:
> On Sunday 23 March 2014 00:33:04 Matt Whitlock wrote:
> > Attached are a series of three patches affecting swaponoff.
>
> Hi,
> attached you will find a alternative patch to your patches #1 and #2 that
> includes your fixes and
> improvements and reworks swap_on_off_main to allow you to implement patch #3
> without
> creating a #ifdef hell.
> I hope that the code is also easier readable than the original
> unpatched version.
So are the main improvements here the use of the IF_FEATURE* macros and testing
the ENABLE_FEATURE* macros at the C level rather than at the preprocessor
level? This is still going to create a "hell" when adding the discard flag.
Consider what will happen in the enum: if DISCARD is enabled but PRI is
disabled, then OPT_d will be (1 << 1), but if both are enabled, then OPT_p will
be (1 << 1) and OPT_d will be (1 << 2) (or vice versa). Imagine adding five
more optional features. It leads to a combinatorial explosion. It's cleaner to
downshift the bitmap after checking each bit within its appropriate #if section.
I do agree that I could clean up the construction of the options string using
the IF_FEATURE* macros. See attached patch.>From 0181ba7e95785a85a161160e2624dcd47da7a1c4 Mon Sep 17 00:00:00 2001
From: Matt Whitlock
Date: Sun, 23 Mar 2014 18:06:56 -0400
Subject: [PATCH] improve code readability using IF_FEATURE* macros
---
util-linux/swaponoff.c | 24 ++--
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/util-linux/swaponoff.c b/util-linux/swaponoff.c
index a7ad6db..785d57b 100644
--- a/util-linux/swaponoff.c
+++ b/util-linux/swaponoff.c
@@ -161,12 +161,8 @@ int swap_on_off_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int swap_on_off_main(int argc UNUSED_PARAM, char **argv)
{
int ret;
-#if ENABLE_FEATURE_SWAPON_DISCARD
- char *discard = NULL;
-#endif
-#if ENABLE_FEATURE_SWAPON_PRI
- unsigned prio;
-#endif
+ IF_FEATURE_SWAPON_DISCARD(char *discard = NULL;)
+ IF_FEATURE_SWAPON_PRI(unsigned prio;)
INIT_G();
@@ -178,19 +174,11 @@ int swap_on_off_main(int argc UNUSED_PARAM, char **argv)
opt_complementary = "p+";
#endif
ret = getopt32(argv, (applet_name[5] == 'n') ?
-#if ENABLE_FEATURE_SWAPON_DISCARD
- "d::"
-#endif
-#if ENABLE_FEATURE_SWAPON_PRI
- "p:"
-#endif
+ IF_FEATURE_SWAPON_DISCARD("d::")
+ IF_FEATURE_SWAPON_PRI("p:")
"a" : "a"
-#if ENABLE_FEATURE_SWAPON_DISCARD
- , &discard
-#endif
-#if ENABLE_FEATURE_SWAPON_PRI
- , &prio
-#endif
+ IF_FEATURE_SWAPON_DISCARD(, &discard)
+ IF_FEATURE_SWAPON_PRI(, &prio)
);
#endif
--
1.9.1
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] add discard option -d to swapon
Attached are a series of three patches affecting swaponoff.
Patch #1 fixes two small logic errors: (a) the bb_strtou function was called
inside an instantiation of the MIN macro, which would cause the function to be
called twice in the common case; and (b) the actual maximum allowable swap
priority is SWAP_FLAG_PRIO_MASK >> SWAP_FLAG_PRIO_SHIFT, not simply
SWAP_FLAG_PRIO_MASK as the code previously assumed. It just so happens that
SWAP_FLAG_PRIO_SHIFT == 0, but one should not rely on that serendipity.
Patch #2 fixes the interaction of the -a and -p options to swapon. When -p is
given on the command line, the specified priority should be applied to any swap
areas given in /etc/fstab that lack a 'pri' option. This is the way swapon from
util-linux does it, and it's the rational behavior, or else -p and -a should be
mutually exclusive options.
Patch #3 adds a -d option to swapon, which sets SWAP_FLAG_DISCARD and
potentially SWAP_FLAG_DISCARD_ONCE or SWAP_FLAG_DISCARD_PAGES if a policy is
given. The patch also adds support for the 'discard' option in swap entries in
/etc/fstab.>From 1509caf8e5088ed92ae6c7ab54d65785df570351 Mon Sep 17 00:00:00 2001
From: Matt Whitlock
Date: Sat, 22 Mar 2014 18:54:24 -0400
Subject: [PATCH 1/3] avoid calling bb_strtou twice in MIN macro expansion
Also, the maximum allowable value of swap priority is technically SWAP_FLAG_PRIO_MASK >> SWAP_FLAG_PRIO_SHIFT.
---
util-linux/swaponoff.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/util-linux/swaponoff.c b/util-linux/swaponoff.c
index 3f22334..d35e30e 100644
--- a/util-linux/swaponoff.c
+++ b/util-linux/swaponoff.c
@@ -100,12 +100,12 @@ static int do_em_all(void)
g_flags = 0; /* each swap space might have different flags */
p = hasmntopt(m, "pri");
if (p) {
- /* Max allowed 32767 (==SWAP_FLAG_PRIO_MASK) */
- unsigned int swap_prio = MIN(bb_strtou(p + 4 , NULL, 10), SWAP_FLAG_PRIO_MASK);
+ /* Max allowed 32767 (== SWAP_FLAG_PRIO_MASK >> SWAP_FLAG_PRIO_SHIFT) */
+ unsigned prio = bb_strtou(p + 4, NULL, 10);
/* We want to allow ",foo", thus errno == EINVAL is allowed too */
if (errno != ERANGE) {
g_flags = SWAP_FLAG_PREFER |
- (swap_prio << SWAP_FLAG_PRIO_SHIFT);
+ MIN(prio, SWAP_FLAG_PRIO_MASK >> SWAP_FLAG_PRIO_SHIFT) << SWAP_FLAG_PRIO_SHIFT;
}
}
#endif
@@ -124,6 +124,9 @@ int swap_on_off_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int swap_on_off_main(int argc UNUSED_PARAM, char **argv)
{
int ret;
+#if ENABLE_FEATURE_SWAPON_PRI
+ unsigned prio;
+#endif
INIT_G();
@@ -132,11 +135,11 @@ int swap_on_off_main(int argc UNUSED_PARAM, char **argv)
#else
if (applet_name[5] == 'n')
opt_complementary = "p+";
- ret = getopt32(argv, (applet_name[5] == 'n') ? "ap:" : "a", &g_flags);
+ ret = getopt32(argv, (applet_name[5] == 'n') ? "ap:" : "a", &prio);
if (ret & 2) { // -p
g_flags = SWAP_FLAG_PREFER |
- ((g_flags & SWAP_FLAG_PRIO_MASK) << SWAP_FLAG_PRIO_SHIFT);
+ MIN(prio, SWAP_FLAG_PRIO_MASK >> SWAP_FLAG_PRIO_SHIFT) << SWAP_FLAG_PRIO_SHIFT;
ret &= 1;
}
#endif
--
1.9.1
>From e7d0d02ca14aea0e12c2ca6b84e124db25172209 Mon Sep 17 00:00:00 2001
From: Matt Whitlock
Date: Sat, 22 Mar 2014 19:10:08 -0400
Subject: [PATCH 2/3] fix interaction of -a and -p options in swapon
Swap entries in /etc/fstab inherit the priority specified on the command line unless they have 'pri' in their mount options.
---
util-linux/swaponoff.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/util-linux/swaponoff.c b/util-linux/swaponoff.c
index d35e30e..df0c524 100644
--- a/util-linux/swaponoff.c
+++ b/util-linux/swaponoff.c
@@ -82,6 +82,9 @@ static int do_em_all(void)
struct mntent *m;
FILE *f;
int err;
+#ifdef G
+ int cl_flags = g_flags;
+#endif
f = setmntent("/etc/fstab", "r");
if (f == NULL)
@@ -97,14 +100,14 @@ static int do_em_all(void)
) {
#if ENABLE_FEATURE_SWAPON_PRI
char *p;
-g_flags = 0; /* each swap space might have different flags */
+g_flags = cl_flags; /* each swap space might have different flags */
p = hasmntopt(m, "pri");
if (p) {
/* Max allowed 32767 (== SWAP_FLAG_PRIO_MASK >> SWAP_FLAG_PRIO_SHIFT) */
unsigned prio = bb_strtou(p + 4, NULL, 10);
/* We want to allow ",foo", thus errno == EINVAL is allowed too */
if (errno != ERANGE) {
- g_flags = SWAP_FLAG_PREFER |
+ g_flags = (g_flags & ~SWAP_FLAG_PRIO_MASK) | SWAP_FLAG_PREFER |
MIN(prio, SWAP_FLAG_PRIO_MASK >> SWAP_FLAG_PRIO_SHIFT) << SWAP_FLAG_PRIO_SHIFT;
}
}
--
1.9.1
>From b2b2e65829785c959881d643bcff8843a6cddf7e Mon Sep 17 00:0
