[PATCH] function bb_askpass only read one line once

Michael Zhu linuxsir320 at gmail.com
Mon Feb 1 09:13:05 UTC 2010


Hi Denys Vlasenko,

I'm sorry to reply to wrong subject. Post here again.

Yours is easy to understand. Thanks for your feedback.

+       /* discards \n and \r */
+       while((len = read(fd,&ch, 1)>  0)&&  (ch == '\n' || ch == '\r'));

Reading is terminated when encountering '\r', but DOS file format ends 
with newline '\r\n'.
I wan to get the next line, so '\n' is dropped. I known this 
modification is not better, but
I could not find a better way. How about this: saving the character 
read, when reading next
time, if character is '\n', and last character just saved is '\r', 
skipping it and going on. Maybe
it's not necessary.

diff --git a/libbb/bb_askpass.c b/libbb/bb_askpass.c
index f9b918c..2792c89 100644
--- a/libbb/bb_askpass.c
+++ b/libbb/bb_askpass.c
@@ -24,16 +24,13 @@ char* FAST_FUNC bb_ask(const int fd, int timeout, 
const char *prompt)
      /* Was static char[BIGNUM] */
      enum { sizeof_passwd = 128 };
      static char *passwd;
+    static char last_char = 0;

      char *ret;
      int i;
      struct sigaction sa, oldsa;
      struct termios tio, oldtio;

-    if (!passwd)
-        passwd = xmalloc(sizeof_passwd);
-    memset(passwd, 0, sizeof_passwd);
-
      tcgetattr(fd, &oldtio);
      tcflush(fd, TCIFLUSH);
      tio = oldtio;
@@ -46,7 +43,7 @@ char* FAST_FUNC bb_ask(const int fd, int timeout, 
const char *prompt)

      memset(&sa, 0, sizeof(sa));
      /* sa.sa_flags = 0; - no SA_RESTART! */
-    /* SIGINT and SIGALRM will interrupt read below */
+    /* SIGINT and SIGALRM will interrupt reads below */
      sa.sa_handler = askpass_timeout;
      sigaction(SIGINT, &sa, &oldsa);
      if (timeout) {
@@ -56,18 +53,33 @@ char* FAST_FUNC bb_ask(const int fd, int timeout, 
const char *prompt)

      fputs(prompt, stdout);
      fflush_all();
-    ret = NULL;
-    /* On timeout or Ctrl-C, read will hopefully be interrupted,
-     * and we return NULL */
-    if (read(fd, passwd, sizeof_passwd - 1) > 0) {
-        ret = passwd;
-        i = 0;
-        /* Last byte is guaranteed to be 0
-           (read did not overwrite it) */
-        do {
-            if (passwd[i] == '\r' || passwd[i] == '\n')
-                passwd[i] = '\0';
-        } while (passwd[i++]);
+
+    if (!passwd)
+        passwd = xmalloc(sizeof_passwd);
+    memset(passwd, 0, sizeof_passwd);
+    ret = passwd;
+    i = 0;
+    while (1) {
+        int r = read(fd, &ret[i], 1);
+        if (r < 0) {
+            /* read is interrupted by timeout or ^C */
+            ret = NULL;
+            break;
+        }
+        if (r==1)
+        {
+            if (last_char == '\r' && ret[i] == '\n')
+                continue;
+            last_char = ret[i];
+        }
+
+        if (r == 0 /* EOF */
+         || ret[i] == '\r' || ret[i] == '\n' /* EOL */
+         || ++i == sizeof_passwd-1 /* line limit */
+        ) {
+            ret[i] = '\0';
+            break;
+        }
      }

      if (timeout) {


Thanks again!

Michael
On 2/1/2010 11:56 AM, Denys Vlasenko wrote:
> On Friday 29 January 2010 06:42, Michael Zhu wrote:
>    
>> New patch for latest developing busybox. After applying patch attached,
>> passwd works as standard GNU/Linux.
>> I'm looking forward your comment.
>>      
> It is necessary to not simply send a patch, like you did
> in the first email, but to give the example where patch helps.
>
> Thanks for doing it now:
>
>    
>> # passwd<<EOF
>>   >  Greatbusybox
>>   >  Greatbusybox
>>   >  EOF
>> Changing password for root
>> New password:
>> Retype password:
>> Password for root changed by root
>> #
>> # echo -e "Busybox\nBusybox"|passwd
>> Changing password for root
>> New password:
>> Bad password: too weak
>> Retype password:
>> Password for root changed by root
>> #
>>      
> Let's look at the patch
>
> +
> +       /* discards \n and \r */
> +       while((len = read(fd,&ch, 1)>  0)&&  (ch == '\n' || ch == '\r'));
>
> Why do you do it?
>
> +               while(i<  sizeof_passwd - 1&&  ( len = read(fd,&ch, 1) )>  0
> +&&  ch != '\n'&&  ch != '\r'){
>
> The code looks needlessly confusing.
>
> +                       /* Last byte is guaranteed to be 0 */
> +                       passwd[i++] = ch;
> +               }
>
> The comment is now misplaced, the thing it is talking about
> is not on passwd[i++] = ch line.
>
> +       /* On timeout or Ctrl-C, read will hopefully be interrupted,
> +        * and we return NULL */
> +       return (len<  0 ? NULL : passwd);
>
> Again, misplaced comment.
>
>
>
> How about this?
>
> diff -ad -urpN busybox.4/libbb/bb_askpass.c busybox.5/libbb/bb_askpass.c
> --- busybox.4/libbb/bb_askpass.c	2010-01-31 18:10:15.000000000 +0100
> +++ busybox.5/libbb/bb_askpass.c	2010-02-01 04:50:52.000000000 +0100
> @@ -30,10 +30,6 @@ char* FAST_FUNC bb_ask(const int fd, int
>   	struct sigaction sa, oldsa;
>   	struct termios tio, oldtio;
>
> -	if (!passwd)
> -		passwd = xmalloc(sizeof_passwd);
> -	memset(passwd, 0, sizeof_passwd);
> -
>   	tcgetattr(fd,&oldtio);
>   	tcflush(fd, TCIFLUSH);
>   	tio = oldtio;
> @@ -46,7 +42,7 @@ char* FAST_FUNC bb_ask(const int fd, int
>
>   	memset(&sa, 0, sizeof(sa));
>   	/* sa.sa_flags = 0; - no SA_RESTART! */
> -	/* SIGINT and SIGALRM will interrupt read below */
> +	/* SIGINT and SIGALRM will interrupt reads below */
>   	sa.sa_handler = askpass_timeout;
>   	sigaction(SIGINT,&sa,&oldsa);
>   	if (timeout) {
> @@ -56,18 +52,26 @@ char* FAST_FUNC bb_ask(const int fd, int
>
>   	fputs(prompt, stdout);
>   	fflush_all();
> -	ret = NULL;
> -	/* On timeout or Ctrl-C, read will hopefully be interrupted,
> -	 * and we return NULL */
> -	if (read(fd, passwd, sizeof_passwd - 1)>  0) {
> -		ret = passwd;
> -		i = 0;
> -		/* Last byte is guaranteed to be 0
> -		   (read did not overwrite it) */
> -		do {
> -			if (passwd[i] == '\r' || passwd[i] == '\n')
> -				passwd[i] = '\0';
> -		} while (passwd[i++]);
> +
> +	if (!passwd)
> +		passwd = xmalloc(sizeof_passwd);
> +	memset(passwd, 0, sizeof_passwd);
> +	ret = passwd;
> +	i = 0;
> +	while (1) {
> +		int r = read(fd,&ret[i], 1);
> +		if (r<  0) {
> +			/* read is interrupted by timeout or ^C */
> +			ret = NULL;
> +			break;
> +		}
> +		if (r == 0 /* EOF */
> +		 || ret[i] == '\r' || ret[i] == '\n' /* EOL */
> +		 || ++i == sizeof_passwd-1 /* line limit */
> +		) {
> +			ret[i] = '\0';
> +			break;
> +		}
>   	}
>
>   	if (timeout) {
>
>    



More information about the busybox mailing list