Re: [PATCH] libbb: remove unnecessary argument to nonblock_immune_read
On Mon, Apr 20, 2015 at 2:35 PM, walter harms wrote: > just for my curiosity: > if only ash uses this function should why it is in libbb ? It was used by hush too. I envision other cases where I'd want to use it. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] libbb: remove unnecessary argument to nonblock_immune_read
walter harms wrote: >just for my curiosity: >if only ash uses this function should why it is in libbb ? Only ash calls it directly. It's also called from xmalloc_reads which is called by patch, mail, lpd and acpid. Ron ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] libbb: remove unnecessary argument to nonblock_immune_read
just for my curiosity:
if only ash uses this function should why it is in libbb ?
re,
wh
Am 20.04.2015 13:52, schrieb Denys Vlasenko:
> Applied, thanks!
>
> On Sun, Apr 19, 2015 at 11:50 AM, Ron Yorston wrote:
>> The loop_on_EINTR argument to nonblock_immune_read is always set to 1.
>>
>> function old new delta
>> xmalloc_reads200 195 -5
>> pgetc488 483 -5
>> argstr 13131308 -5
>> nonblock_immune_read 123 86 -37
>> --
>> (add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-52) Total: -52
>> bytes
>>
>> Signed-off-by: Ron Yorston
>> ---
>> include/libbb.h | 2 +-
>> libbb/read_printf.c | 8
>> shell/ash.c | 6 +++---
>> 3 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/libbb.h b/include/libbb.h
>> index 0f8363b..21da5f1 100644
>> --- a/include/libbb.h
>> +++ b/include/libbb.h
>> @@ -713,7 +713,7 @@ void* xrealloc_vector_helper(void *vector, unsigned
>> sizeof_and_shift, int idx) F
>>
>>
>> extern ssize_t safe_read(int fd, void *buf, size_t count) FAST_FUNC;
>> -extern ssize_t nonblock_immune_read(int fd, void *buf, size_t count, int
>> loop_on_EINTR) FAST_FUNC;
>> +extern ssize_t nonblock_immune_read(int fd, void *buf, size_t count)
>> FAST_FUNC;
>> // NB: will return short read on error, not -1,
>> // if some data was read before error occurred
>> extern ssize_t full_read(int fd, void *buf, size_t count) FAST_FUNC;
>> diff --git a/libbb/read_printf.c b/libbb/read_printf.c
>> index 5ed6e36..b6a17cc 100644
>> --- a/libbb/read_printf.c
>> +++ b/libbb/read_printf.c
>> @@ -45,20 +45,20 @@
>> * which detects EAGAIN and uses poll() to wait on the fd.
>> * Thankfully, poll() doesn't care about O_NONBLOCK flag.
>> */
>> -ssize_t FAST_FUNC nonblock_immune_read(int fd, void *buf, size_t count, int
>> loop_on_EINTR)
>> +ssize_t FAST_FUNC nonblock_immune_read(int fd, void *buf, size_t count)
>> {
>> struct pollfd pfd[1];
>> ssize_t n;
>>
>> while (1) {
>> - n = loop_on_EINTR ? safe_read(fd, buf, count) : read(fd,
>> buf, count);
>> + n = safe_read(fd, buf, count);
>> if (n >= 0 || errno != EAGAIN)
>> return n;
>> /* fd is in O_NONBLOCK mode. Wait using poll and repeat */
>> pfd[0].fd = fd;
>> pfd[0].events = POLLIN;
>> /* note: safe_poll pulls in printf */
>> - loop_on_EINTR ? safe_poll(pfd, 1, -1) : poll(pfd, 1, -1);
>> + safe_poll(pfd, 1, -1);
>> }
>> }
>>
>> @@ -81,7 +81,7 @@ char* FAST_FUNC xmalloc_reads(int fd, size_t *maxsz_p)
>> p = buf + sz;
>> sz += 128;
>> }
>> - if (nonblock_immune_read(fd, p, 1, /*loop_on_EINTR:*/ 1) !=
>> 1) {
>> + if (nonblock_immune_read(fd, p, 1) != 1) {
>> /* EOF/error */
>> if (p == buf) { /* we read nothing */
>> free(buf);
>> diff --git a/shell/ash.c b/shell/ash.c
>> index b568013..6718901 100644
>> --- a/shell/ash.c
>> +++ b/shell/ash.c
>> @@ -5923,7 +5923,7 @@ expbackq(union node *cmd, int quoted, int quotes)
>> read:
>> if (in.fd < 0)
>> break;
>> - i = nonblock_immune_read(in.fd, buf, sizeof(buf),
>> /*loop_on_EINTR:*/ 1);
>> + i = nonblock_immune_read(in.fd, buf, sizeof(buf));
>> TRACE(("expbackq: read returns %d\n", i));
>> if (i <= 0)
>> break;
>> @@ -9677,7 +9677,7 @@ preadfd(void)
>> #if ENABLE_FEATURE_EDITING
>> retry:
>> if (!iflag || g_parsefile->pf_fd != STDIN_FILENO)
>> - nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ -
>> 1, /*loop_on_EINTR:*/ 1);
>> + nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ -
>> 1);
>> else {
>> int timeout = -1;
>> # if ENABLE_ASH_IDLE_TIMEOUT
>> @@ -9719,7 +9719,7 @@ preadfd(void)
>> }
>> }
>> #else
>> - nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1,
>> /*loop_on_EINTR:*/ 1);
>> + nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1);
>> #endif
>>
>> #if 0 /* disabled: nonblock_immune_read() handles this problem */
>> --
>> 2.1.0
>>
>> ___
>> busybox mailing list
>> busybox@busybox.net
>> http://lists.busybox.net/mailman/listinfo/busybox
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman
Re: [PATCH] libbb: remove unnecessary argument to nonblock_immune_read
Applied, thanks!
On Sun, Apr 19, 2015 at 11:50 AM, Ron Yorston wrote:
> The loop_on_EINTR argument to nonblock_immune_read is always set to 1.
>
> function old new delta
> xmalloc_reads200 195 -5
> pgetc488 483 -5
> argstr 13131308 -5
> nonblock_immune_read 123 86 -37
> --
> (add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-52) Total: -52 bytes
>
> Signed-off-by: Ron Yorston
> ---
> include/libbb.h | 2 +-
> libbb/read_printf.c | 8
> shell/ash.c | 6 +++---
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/include/libbb.h b/include/libbb.h
> index 0f8363b..21da5f1 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
> @@ -713,7 +713,7 @@ void* xrealloc_vector_helper(void *vector, unsigned
> sizeof_and_shift, int idx) F
>
>
> extern ssize_t safe_read(int fd, void *buf, size_t count) FAST_FUNC;
> -extern ssize_t nonblock_immune_read(int fd, void *buf, size_t count, int
> loop_on_EINTR) FAST_FUNC;
> +extern ssize_t nonblock_immune_read(int fd, void *buf, size_t count)
> FAST_FUNC;
> // NB: will return short read on error, not -1,
> // if some data was read before error occurred
> extern ssize_t full_read(int fd, void *buf, size_t count) FAST_FUNC;
> diff --git a/libbb/read_printf.c b/libbb/read_printf.c
> index 5ed6e36..b6a17cc 100644
> --- a/libbb/read_printf.c
> +++ b/libbb/read_printf.c
> @@ -45,20 +45,20 @@
> * which detects EAGAIN and uses poll() to wait on the fd.
> * Thankfully, poll() doesn't care about O_NONBLOCK flag.
> */
> -ssize_t FAST_FUNC nonblock_immune_read(int fd, void *buf, size_t count, int
> loop_on_EINTR)
> +ssize_t FAST_FUNC nonblock_immune_read(int fd, void *buf, size_t count)
> {
> struct pollfd pfd[1];
> ssize_t n;
>
> while (1) {
> - n = loop_on_EINTR ? safe_read(fd, buf, count) : read(fd, buf,
> count);
> + n = safe_read(fd, buf, count);
> if (n >= 0 || errno != EAGAIN)
> return n;
> /* fd is in O_NONBLOCK mode. Wait using poll and repeat */
> pfd[0].fd = fd;
> pfd[0].events = POLLIN;
> /* note: safe_poll pulls in printf */
> - loop_on_EINTR ? safe_poll(pfd, 1, -1) : poll(pfd, 1, -1);
> + safe_poll(pfd, 1, -1);
> }
> }
>
> @@ -81,7 +81,7 @@ char* FAST_FUNC xmalloc_reads(int fd, size_t *maxsz_p)
> p = buf + sz;
> sz += 128;
> }
> - if (nonblock_immune_read(fd, p, 1, /*loop_on_EINTR:*/ 1) !=
> 1) {
> + if (nonblock_immune_read(fd, p, 1) != 1) {
> /* EOF/error */
> if (p == buf) { /* we read nothing */
> free(buf);
> diff --git a/shell/ash.c b/shell/ash.c
> index b568013..6718901 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -5923,7 +5923,7 @@ expbackq(union node *cmd, int quoted, int quotes)
> read:
> if (in.fd < 0)
> break;
> - i = nonblock_immune_read(in.fd, buf, sizeof(buf),
> /*loop_on_EINTR:*/ 1);
> + i = nonblock_immune_read(in.fd, buf, sizeof(buf));
> TRACE(("expbackq: read returns %d\n", i));
> if (i <= 0)
> break;
> @@ -9677,7 +9677,7 @@ preadfd(void)
> #if ENABLE_FEATURE_EDITING
> retry:
> if (!iflag || g_parsefile->pf_fd != STDIN_FILENO)
> - nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ -
> 1, /*loop_on_EINTR:*/ 1);
> + nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ -
> 1);
> else {
> int timeout = -1;
> # if ENABLE_ASH_IDLE_TIMEOUT
> @@ -9719,7 +9719,7 @@ preadfd(void)
> }
> }
> #else
> - nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1,
> /*loop_on_EINTR:*/ 1);
> + nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1);
> #endif
>
> #if 0 /* disabled: nonblock_immune_read() handles this problem */
> --
> 2.1.0
>
> ___
> 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] libbb: remove unnecessary argument to nonblock_immune_read
On Sun, Apr 19, 2015 at 6:07 PM, Rich Felker wrote: > On Sun, Apr 19, 2015 at 10:50:25AM +0100, Ron Yorston wrote: >> The loop_on_EINTR argument to nonblock_immune_read is always set to 1. >> >> function old new delta >> xmalloc_reads200 195 -5 >> pgetc488 483 -5 >> argstr 13131308 -5 >> nonblock_immune_read 123 86 -37 >> -- >> (add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-52) Total: -52 >> bytes > > An even better question is whether the looping code could be > eliminated entirely. Are any of the bb commands that use this code > installing interurpting signal handlers? If not then the code for > supporting EINTR is useless. ash uses this function. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] libbb: remove unnecessary argument to nonblock_immune_read
On Sun, Apr 19, 2015 at 10:50:25AM +0100, Ron Yorston wrote: > The loop_on_EINTR argument to nonblock_immune_read is always set to 1. > > function old new delta > xmalloc_reads200 195 -5 > pgetc488 483 -5 > argstr 13131308 -5 > nonblock_immune_read 123 86 -37 > -- > (add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-52) Total: -52 bytes An even better question is whether the looping code could be eliminated entirely. Are any of the bb commands that use this code installing interurpting signal handlers? If not then the code for supporting EINTR is useless. Rich ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] libbb: remove unnecessary argument to nonblock_immune_read
The loop_on_EINTR argument to nonblock_immune_read is always set to 1.
function old new delta
xmalloc_reads200 195 -5
pgetc488 483 -5
argstr 13131308 -5
nonblock_immune_read 123 86 -37
--
(add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-52) Total: -52 bytes
Signed-off-by: Ron Yorston
---
include/libbb.h | 2 +-
libbb/read_printf.c | 8
shell/ash.c | 6 +++---
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/libbb.h b/include/libbb.h
index 0f8363b..21da5f1 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -713,7 +713,7 @@ void* xrealloc_vector_helper(void *vector, unsigned
sizeof_and_shift, int idx) F
extern ssize_t safe_read(int fd, void *buf, size_t count) FAST_FUNC;
-extern ssize_t nonblock_immune_read(int fd, void *buf, size_t count, int
loop_on_EINTR) FAST_FUNC;
+extern ssize_t nonblock_immune_read(int fd, void *buf, size_t count) FAST_FUNC;
// NB: will return short read on error, not -1,
// if some data was read before error occurred
extern ssize_t full_read(int fd, void *buf, size_t count) FAST_FUNC;
diff --git a/libbb/read_printf.c b/libbb/read_printf.c
index 5ed6e36..b6a17cc 100644
--- a/libbb/read_printf.c
+++ b/libbb/read_printf.c
@@ -45,20 +45,20 @@
* which detects EAGAIN and uses poll() to wait on the fd.
* Thankfully, poll() doesn't care about O_NONBLOCK flag.
*/
-ssize_t FAST_FUNC nonblock_immune_read(int fd, void *buf, size_t count, int
loop_on_EINTR)
+ssize_t FAST_FUNC nonblock_immune_read(int fd, void *buf, size_t count)
{
struct pollfd pfd[1];
ssize_t n;
while (1) {
- n = loop_on_EINTR ? safe_read(fd, buf, count) : read(fd, buf,
count);
+ n = safe_read(fd, buf, count);
if (n >= 0 || errno != EAGAIN)
return n;
/* fd is in O_NONBLOCK mode. Wait using poll and repeat */
pfd[0].fd = fd;
pfd[0].events = POLLIN;
/* note: safe_poll pulls in printf */
- loop_on_EINTR ? safe_poll(pfd, 1, -1) : poll(pfd, 1, -1);
+ safe_poll(pfd, 1, -1);
}
}
@@ -81,7 +81,7 @@ char* FAST_FUNC xmalloc_reads(int fd, size_t *maxsz_p)
p = buf + sz;
sz += 128;
}
- if (nonblock_immune_read(fd, p, 1, /*loop_on_EINTR:*/ 1) != 1) {
+ if (nonblock_immune_read(fd, p, 1) != 1) {
/* EOF/error */
if (p == buf) { /* we read nothing */
free(buf);
diff --git a/shell/ash.c b/shell/ash.c
index b568013..6718901 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -5923,7 +5923,7 @@ expbackq(union node *cmd, int quoted, int quotes)
read:
if (in.fd < 0)
break;
- i = nonblock_immune_read(in.fd, buf, sizeof(buf),
/*loop_on_EINTR:*/ 1);
+ i = nonblock_immune_read(in.fd, buf, sizeof(buf));
TRACE(("expbackq: read returns %d\n", i));
if (i <= 0)
break;
@@ -9677,7 +9677,7 @@ preadfd(void)
#if ENABLE_FEATURE_EDITING
retry:
if (!iflag || g_parsefile->pf_fd != STDIN_FILENO)
- nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1,
/*loop_on_EINTR:*/ 1);
+ nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1);
else {
int timeout = -1;
# if ENABLE_ASH_IDLE_TIMEOUT
@@ -9719,7 +9719,7 @@ preadfd(void)
}
}
#else
- nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1,
/*loop_on_EINTR:*/ 1);
+ nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1);
#endif
#if 0 /* disabled: nonblock_immune_read() handles this problem */
--
2.1.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
