date:20171018

[PATCH v2 1/2] time applet: fix a build problem with kernel versions missing O_CLOEXEC symbol

2017-10-18 Thread Eugene Rudoy

Kernel versions < 2.6.23 do not support/provide O_CLOEXEC symbol
causing the time applet not to compile:

 time.c: In function 'time_main':
 time.c:445:28: error: 'O_CLOEXEC' undeclared (first use in this function)
 time.c:445:28: note: each undeclared identifier is reported only once for each 
function it appears in

Fix it by using close_on_exec_on function provided by libbb
instead of passing O_CLOEXEC flag to open.

Signed-off-by: Eugene Rudoy 
---

 v2: remove unnecessary "if (output_fd >= 0)", xopen guarantees the returned
 file descriptor to be valid, thanks to Xabier Oneca for pointing it out
---
 miscutils/time.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/miscutils/time.c b/miscutils/time.c
index f4f8149d3..c40d945e6 100644
--- a/miscutils/time.c
+++ b/miscutils/time.c
@@ -444,9 +444,10 @@ int time_main(int argc UNUSED_PARAM, char **argv)
if (opt & OPT_o) {
output_fd = xopen(output_filename,
(opt & OPT_a) /* append? */
-   ? (O_CREAT | O_WRONLY | O_CLOEXEC | O_APPEND)
-   : (O_CREAT | O_WRONLY | O_CLOEXEC | O_TRUNC)
+   ? (O_CREAT | O_WRONLY | O_APPEND)
+   : (O_CREAT | O_WRONLY | O_TRUNC)
);
+   close_on_exec_on(output_fd);
}
 
run_command(argv, &res);
-- 
2.14.2
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH v2 2/2] iproute/iprule: restore support for kernel versions missing RTA_TABLE routing attribute

2017-10-18 Thread Eugene Rudoy

iproute/iprule applets fail to compile when compiled using kernel versions < 
2.6.19

 iproute.c: In function 'print_route':
 iproute.c:85:9: error: 'RTA_TABLE' undeclared (first use in this function)
 iproute.c:85:9: note: each undeclared identifier is reported only once for 
each function it appears in
 iproute.c: In function 'iproute_modify':
 iproute.c:467:36: error: 'RTA_TABLE' undeclared (first use in this function)

Fix it by partially #ifdef'ing the code added in 
b42107f21538e39d9a344376372f8261aed589b2

Signed-off-by: Eugene Rudoy 
---

 v2: change the order of if/else-branches to reduce the number of #ifdef's
 as suggested by Walter Harms
---
 networking/libiproute/iproute.c | 14 ++
 networking/libiproute/iprule.c  | 19 ++-
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/networking/libiproute/iproute.c b/networking/libiproute/iproute.c
index e8b26cb2f..9b3d9c3b8 100644
--- a/networking/libiproute/iproute.c
+++ b/networking/libiproute/iproute.c
@@ -14,6 +14,9 @@
 #include "rt_names.h"
 #include "utils.h"
 
+#include 
+#define HAVE_RTA_TABLE (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,19))
+
 #ifndef RTAX_RTTVAR
 #define RTAX_RTTVAR RTAX_HOPS
 #endif
@@ -81,9 +84,11 @@ static int FAST_FUNC print_route(const struct sockaddr_nl 
*who UNUSED_PARAM,
memset(tb, 0, sizeof(tb));
parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
 
+#if HAVE_RTA_TABLE
if (tb[RTA_TABLE])
tid = *(uint32_t *)RTA_DATA(tb[RTA_TABLE]);
else
+#endif
tid = r->rtm_table;
 
if (r->rtm_family == AF_INET6)
@@ -459,12 +464,13 @@ IF_FEATURE_IP_RULE(ARG_table,)
NEXT_ARG();
if (rtnl_rttable_a2n(&tid, *argv))
invarg_1_to_2(*argv, keyword_table);
-   if (tid < 256)
-   req.r.rtm_table = tid;
-   else {
+#if HAVE_RTA_TABLE
+   if (tid > 255) {
req.r.rtm_table = RT_TABLE_UNSPEC;
addattr32(&req.n, sizeof(req), RTA_TABLE, tid);
-   }
+   } else
+#endif
+   req.r.rtm_table = tid;
 #endif
} else if (arg == ARG_dev || arg == ARG_oif) {
NEXT_ARG();
diff --git a/networking/libiproute/iprule.c b/networking/libiproute/iprule.c
index 9938b4793..772890982 100644
--- a/networking/libiproute/iprule.c
+++ b/networking/libiproute/iprule.c
@@ -24,6 +24,9 @@
 #include "rt_names.h"
 #include "utils.h"
 
+#include 
+#define HAVE_RTA_TABLE (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,19))
+
 /* If you add stuff here, update iprule_full_usage */
 static const char keywords[] ALIGN1 =
"from\0""to\0""preference\0""order\0""priority\0"
@@ -120,9 +123,12 @@ static int FAST_FUNC print_rule(const struct sockaddr_nl 
*who UNUSED_PARAM,
printf("iif %s ", (char*)RTA_DATA(tb[RTA_IIF]));
}
 
+#if HAVE_RTA_TABLE
if (tb[RTA_TABLE])
printf("lookup %s ", 
rtnl_rttable_n2a(*(uint32_t*)RTA_DATA(tb[RTA_TABLE])));
-   else if (r->rtm_table)
+   else
+#endif
+   if (r->rtm_table)
printf("lookup %s ", rtnl_rttable_n2a(r->rtm_table));
 
if (tb[FRA_SUPPRESS_PREFIXLEN]) {
@@ -266,12 +272,15 @@ static int iprule_modify(int cmd, char **argv)
NEXT_ARG();
if (rtnl_rttable_a2n(&tid, *argv))
invarg_1_to_2(*argv, "table ID");
-   if (tid < 256)
-   req.r.rtm_table = tid;
-   else {
+
+#if HAVE_RTA_TABLE
+   if (tid > 255) {
req.r.rtm_table = RT_TABLE_UNSPEC;
addattr32(&req.n, sizeof(req), RTA_TABLE, tid);
-   }
+   } else
+#endif
+   req.r.rtm_table = tid;
+
table_ok = 1;
} else if (key == ARG_suppress_prefixlength) {
int prefix_length;
-- 
2.14.2
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/2] fix O_CLOEXEC related build problems with kernel versions < 2.6.23

2017-10-18 Thread Eugene Rudoy

Hi Ralf,

On Wed, Oct 18, 2017 at 12:31 PM, Ralf Friedl  wrote:
> What is the purpose of O_CLOEXEC here? In both cases, we try to open a file.
> If the file is opened, we use the finit_module syscall and immediately close
> the file.  Is there a danger that finit_module might exec something and the
> file needs to be closed if that happens?
> Specifying O_CLOEXEC as a parameter to open just changes the value of the
> constant, although loading a large value like O_CLOEXEC needs an extra
> instruction on MIPS, but this adds an additional function call, so is it
> really needed?

Both changes within

# ifdef __NR_finit_module
...
#endif

are unnecessary. finit_module is available since kernel version 3.8,
so the code will only be executed for kernel versions definitely
supporting/providing O_CLOEXEC. I will omit the corresponding hunks
from v2 of my patch.

Denys can remove these two O_CLOEXEC's if he considers them being
unnecessary, but keeping them doesn't harm either.

Cheers,
Gene
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/2] fix O_CLOEXEC related build problems with kernel versions < 2.6.23

2017-10-18 Thread Eugene Rudoy

Hi Walter,

On Wed, Oct 18, 2017 at 6:27 PM, walter harms  wrote:
> while you are there ...
> please be more verbose "Do not use O_CLOEXEC flags" is nice but why ?
hmm, as the subject line states the change fixes the *build* problems
with *kernel* versions < 2.6.23, e.g.

miscutils/time.c: In function 'time_main':
miscutils/time.c:445:28: error: 'O_CLOEXEC' undeclared (first use in
this function)
miscutils/time.c:445:28: note: each undeclared identifier is reported
only once for each function it appears in

> You use a libc that das not support O_CLOEXEC ?, does it cause a bug later ? 
> 
a *kernel* version not supporting it and thus not providing the
corresponding symbol

Cheers,
Gene
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/2] fix O_CLOEXEC related build problems with kernel versions < 2.6.23

2017-10-18 Thread walter harms



Am 18.10.2017 18:23, schrieb Eugene Rudoy:
> Hi Xabier,
> 
> On Wed, Oct 18, 2017 at 8:52 AM, Xabier Oneca -- xOneca
>  wrote:
>>
>> Doesn't xopen always guarantee output_fd will be valid? If so, there's
>> no need for this 'if'.
> you're right, s. [1]. Didn't know that before, thanks for pointing it
> out. v2 of the patch will follow.
> 

while you are there ...
please be more verbose "Do not use O_CLOEXEC flags" is nice but why ?
You use a libc that das not support O_CLOEXEC ?, does it cause a bug later ? 


just a hint,

re,
 wh


> Cheers,
> Gene
> 
> [1] 
> https://git.busybox.net/busybox/tree/libbb/xfuncs_printf.c?id=ebe6d9d8758d36e03cf39b6587597c67ab778436#n129
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
> 
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/2] fix O_CLOEXEC related build problems with kernel versions < 2.6.23

2017-10-18 Thread Eugene Rudoy

Hi Xabier,

On Wed, Oct 18, 2017 at 8:52 AM, Xabier Oneca -- xOneca
 wrote:
>
> Doesn't xopen always guarantee output_fd will be valid? If so, there's
> no need for this 'if'.
you're right, s. [1]. Didn't know that before, thanks for pointing it
out. v2 of the patch will follow.

Cheers,
Gene

[1] 
https://git.busybox.net/busybox/tree/libbb/xfuncs_printf.c?id=ebe6d9d8758d36e03cf39b6587597c67ab778436#n129
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/2] fix O_CLOEXEC related build problems with kernel versions < 2.6.23

2017-10-18 Thread Ralf Friedl


Eugene Rudoy wrote:

Do not use O_CLOEXEC flags (available since 2.6.23),
use close_on_exec_on provided by libbb instead (like everywhere else)

Signed-off-by: Eugene Rudoy 
---
  miscutils/time.c  | 6 --
  modutils/modprobe-small.c | 3 ++-
  modutils/modutils.c   | 3 ++-
  3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
index a94b0b9a6..e782d687d 100644
--- a/modutils/modprobe-small.c
+++ b/modutils/modprobe-small.c
@@ -270,8 +270,9 @@ static int load_module(const char *fname, const char 
*options)
r = 1;
  # ifdef __NR_finit_module
{
-   int fd = open(fname, O_RDONLY | O_CLOEXEC);
+   int fd = open(fname, O_RDONLY);
if (fd >= 0) {
+   close_on_exec_on(fd);
r = finit_module(fd, options, 0) != 0;
close(fd);
}
diff --git a/modutils/modutils.c b/modutils/modutils.c
index 6f7cd9721..9f73d676c 100644
--- a/modutils/modutils.c
+++ b/modutils/modutils.c
@@ -215,8 +215,9 @@ int FAST_FUNC bb_init_module(const char *filename, const 
char *options)
 */
  # ifdef __NR_finit_module
{
-   int fd = open(filename, O_RDONLY | O_CLOEXEC);
+   int fd = open(filename, O_RDONLY);
if (fd >= 0) {
+   close_on_exec_on(fd);
rc = finit_module(fd, options, 0) != 0;
close(fd);
if (rc == 0)
What is the purpose of O_CLOEXEC here? In both cases, we try to open a 
file. If the file is opened, we use the finit_module syscall and 
immediately close the file.  Is there a danger that finit_module might 
exec something and the file needs to be closed if that happens?
Specifying O_CLOEXEC as a parameter to open just changes the value of 
the constant, although loading a large value like O_CLOEXEC needs an 
extra instruction on MIPS, but this adds an additional function call, so 
is it really needed?

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 2/2] iproute/iprule: restore support for kernel versions < 2.6.19

2017-10-18 Thread walter harms



Am 17.10.2017 23:33, schrieb Eugene Rudoy:
> broken in b42107f21538e39d9a344376372f8261aed589b2
> 
> Signed-off-by: Eugene Rudoy 
> ---
>  networking/libiproute/iproute.c |  9 +
>  networking/libiproute/iprule.c  | 12 +++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/networking/libiproute/iproute.c b/networking/libiproute/iproute.c
> index e8b26cb2f..f52398392 100644
> --- a/networking/libiproute/iproute.c
> +++ b/networking/libiproute/iproute.c
> @@ -14,6 +14,9 @@
>  #include "rt_names.h"
>  #include "utils.h"
>  
> +#include 
> +#define HAVE_RTA_TABLE (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,19))
> +
>  #ifndef RTAX_RTTVAR
>  #define RTAX_RTTVAR RTAX_HOPS
>  #endif
> @@ -81,9 +84,11 @@ static int FAST_FUNC print_route(const struct sockaddr_nl 
> *who UNUSED_PARAM,
>   memset(tb, 0, sizeof(tb));
>   parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
>  
> +#if HAVE_RTA_TABLE
>   if (tb[RTA_TABLE])
>   tid = *(uint32_t *)RTA_DATA(tb[RTA_TABLE]);
>   else
> +#endif
>   tid = r->rtm_table;
>  
>   if (r->rtm_family == AF_INET6)
> @@ -459,12 +464,16 @@ IF_FEATURE_IP_RULE(ARG_table,)
>   NEXT_ARG();
>   if (rtnl_rttable_a2n(&tid, *argv))
>   invarg_1_to_2(*argv, keyword_table);
> +#if HAVE_RTA_TABLE
>   if (tid < 256)
> +#endif
>   req.r.rtm_table = tid;
> +#if HAVE_RTA_TABLE
>   else {
>   req.r.rtm_table = RT_TABLE_UNSPEC;
>   addattr32(&req.n, sizeof(req), RTA_TABLE, tid);
>   }
> +#endif

in this special case i would suggest:

#if HAVE_RTA_TABLE
if (tid > 255) {
req.r.rtm_table = RT_TABLE_UNSPEC;
addattr32(&req.n, sizeof(req), RTA_TABLE, tid);
req.r.rtm_table = tid;
} else
#endif
 req.r.rtm_table = tid;


(reduces the #if forrest)



>  #endif
>   } else if (arg == ARG_dev || arg == ARG_oif) {
>   NEXT_ARG();
> diff --git a/networking/libiproute/iprule.c b/networking/libiproute/iprule.c
> index 9938b4793..dabc476c1 100644
> --- a/networking/libiproute/iprule.c
> +++ b/networking/libiproute/iprule.c
> @@ -24,6 +24,9 @@
>  #include "rt_names.h"
>  #include "utils.h"
>  
> +#include 
> +#define HAVE_RTA_TABLE (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,19))
> +
>  /* If you add stuff here, update iprule_full_usage */
>  static const char keywords[] ALIGN1 =
>   "from\0""to\0""preference\0""order\0""priority\0"
> @@ -120,9 +123,12 @@ static int FAST_FUNC print_rule(const struct sockaddr_nl 
> *who UNUSED_PARAM,
>   printf("iif %s ", (char*)RTA_DATA(tb[RTA_IIF]));
>   }
>  
> +#if HAVE_RTA_TABLE
>   if (tb[RTA_TABLE])
>   printf("lookup %s ", 
> rtnl_rttable_n2a(*(uint32_t*)RTA_DATA(tb[RTA_TABLE])));
> - else if (r->rtm_table)
> + else
> +#endif
> + if (r->rtm_table)
>   printf("lookup %s ", rtnl_rttable_n2a(r->rtm_table));
>  
>   if (tb[FRA_SUPPRESS_PREFIXLEN]) {
> @@ -266,12 +272,16 @@ static int iprule_modify(int cmd, char **argv)
>   NEXT_ARG();
>   if (rtnl_rttable_a2n(&tid, *argv))
>   invarg_1_to_2(*argv, "table ID");
> +#if HAVE_RTA_TABLE
>   if (tid < 256)
> +#endif
>   req.r.rtm_table = tid;
> +#if HAVE_RTA_TABLE
>   else {
>   req.r.rtm_table = RT_TABLE_UNSPEC;
>   addattr32(&req.n, sizeof(req), RTA_TABLE, tid);
>   }
> +#endif


see above, can this become a function ?

re,
 wh

>   table_ok = 1;
>   } else if (key == ARG_suppress_prefixlength) {
>   int prefix_length;
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH v2 1/2] time applet: fix a build problem with kernel versions missing O_CLOEXEC symbol

[PATCH v2 2/2] iproute/iprule: restore support for kernel versions missing RTA_TABLE routing attribute

Re: [PATCH 1/2] fix O_CLOEXEC related build problems with kernel versions < 2.6.23

Re: [PATCH 1/2] fix O_CLOEXEC related build problems with kernel versions < 2.6.23

Re: [PATCH 1/2] fix O_CLOEXEC related build problems with kernel versions < 2.6.23

Re: [PATCH 1/2] fix O_CLOEXEC related build problems with kernel versions < 2.6.23

Re: [PATCH 1/2] fix O_CLOEXEC related build problems with kernel versions < 2.6.23

Re: [PATCH 2/2] iproute/iprule: restore support for kernel versions < 2.6.19

8 matches

Site Navigation

Mail list logo

Footer information