Re: [PATCH 1/7] Correct logic in cut_crlf()

2013-02-23 Thread Simon Horman 
On Wed, Feb 20, 2013 at 06:33:08AM +0100, Krzysztof Olędzki wrote:
 On 2013-02-13 09:48, Simon Horman wrote:
 CUT
 
 And add:
 while (*s == '\r' || *s == '\n')
 before *s++ = '\0';
 
 .. so we properly split line1\r\nline2.
 
 I see cut_crlf used to truncate strings at the end of the first line,
 not split them into multiple lines. So I'm not sure if the while()
 loop you suggest is necessary. Are there cases where it is used to split
 strings into lines?
 
 Not yet, and actually, a while loop is not correct as we probably do
 not want to skip silently empty lines. OK, I do not want to block
 you on this important change discussing non-existing usage cases.
 
 The patch looks good to me. Thanks!

Thanks!

Re: [PATCH 1/7] Correct logic in cut_crlf()

2013-02-19 Thread Krzysztof Olędzki 

On 2013-02-13 09:48, Simon Horman wrote:
CUT


And add:
while (*s == '\r' || *s == '\n')
before *s++ = '\0';

.. so we properly split line1\r\nline2.


I see cut_crlf used to truncate strings at the end of the first line,
not split them into multiple lines. So I'm not sure if the while()
loop you suggest is necessary. Are there cases where it is used to split
strings into lines?


Not yet, and actually, a while loop is not correct as we probably do not 
want to skip silently empty lines. OK, I do not want to block you on 
this important change discussing non-existing usage cases.


The patch looks good to me. Thanks!


From: Simon Horman ho...@verge.net.au

This corrects what appears to be logic errors in cut_crlf().
I assume that the intention of this function is to truncate a
string at the first cr or lf. However, currently lf are ignored.

Also use '\0' instead of 0 as the null character, a cosmetic change.

Cc: Krzysztof Piotr Oledzki o...@ans.pl
Signed-off-by: Simon Horman ho...@verge.net.au

---

v2
* Use '\0' instead of 0 as the null character
---
  include/common/standard.h |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/common/standard.h b/include/common/standard.h
index b1db821..6946ded 100644
--- a/include/common/standard.h
+++ b/include/common/standard.h
@@ -411,14 +411,14 @@ unsigned int inetaddr_host_lim_ret(char *text, char 
*stop, char **ret);

  static inline char *cut_crlf(char *s) {

-   while (*s != '\r' || *s == '\n') {
+   while (*s != '\r'  *s != '\n') {
char *p = s++;

if (!*p)
return p;
}

-   *s++ = 0;
+   *s++ = '\0';

return s;
  }



Best regards,

Krzysztof Olędzki

Re: [PATCH 1/7] Correct logic in cut_crlf()

2013-02-13 Thread Simon Horman 
On Tue, Feb 12, 2013 at 04:03:14AM +0100, Krzysztof Olędzki wrote:
 On 2013-02-12 02:45, Simon Horman wrote:
 This corrects what appears to be logic errors in cut_crlf().
 I assume that the intention of this function is to truncate a
 string at the first cr or lf. However, currently lf are ignored.
 
 If the current logic is intended then it may be simplified as:
 
  while (*s != '\r') {
 
 Cc: Krzysztof Piotr Oledzki o...@ans.pl
 Signed-off-by: Simon Horman ho...@verge.net.au
 ---
   include/common/standard.h |2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/include/common/standard.h b/include/common/standard.h
 index b1db821..d931925 100644
 --- a/include/common/standard.h
 +++ b/include/common/standard.h
 @@ -411,7 +411,7 @@ unsigned int inetaddr_host_lim_ret(char *text, char 
 *stop, char **ret);
 
   static inline char *cut_crlf(char *s) {
 
 -while (*s != '\r' || *s == '\n') {
 +while (*s != '\r'  *s != '\n') {
  char *p = s++;
 
  if (!*p)
 
 
 Wow, who wrote this!? ;) Yes, we definitely need this fix, thank you!
 
 Also, you may want to replace:
   *s++ = 0;
 with:
   *s++ = '\0';

Sure, I've provided an updated patch below.

 
 And add:
   while (*s == '\r' || *s == '\n')
 before *s++ = '\0';
 
 .. so we properly split line1\r\nline2.

I see cut_crlf used to truncate strings at the end of the first line,
not split them into multiple lines. So I'm not sure if the while()
loop you suggest is necessary. Are there cases where it is used to split
strings into lines?

From: Simon Horman ho...@verge.net.au

This corrects what appears to be logic errors in cut_crlf().
I assume that the intention of this function is to truncate a
string at the first cr or lf. However, currently lf are ignored.

Also use '\0' instead of 0 as the null character, a cosmetic change.

Cc: Krzysztof Piotr Oledzki o...@ans.pl
Signed-off-by: Simon Horman ho...@verge.net.au

---

v2
* Use '\0' instead of 0 as the null character
---
 include/common/standard.h |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/common/standard.h b/include/common/standard.h
index b1db821..6946ded 100644
--- a/include/common/standard.h
+++ b/include/common/standard.h
@@ -411,14 +411,14 @@ unsigned int inetaddr_host_lim_ret(char *text, char 
*stop, char **ret);
 
 static inline char *cut_crlf(char *s) {
 
-   while (*s != '\r' || *s == '\n') {
+   while (*s != '\r'  *s != '\n') {
char *p = s++;
 
if (!*p)
return p;
}
 
-   *s++ = 0;
+   *s++ = '\0';
 
return s;
 }
-- 
1.7.10.4

[PATCH 1/7] Correct logic in cut_crlf()

2013-02-11 Thread Simon Horman 
This corrects what appears to be logic errors in cut_crlf().
I assume that the intention of this function is to truncate a
string at the first cr or lf. However, currently lf are ignored.

If the current logic is intended then it may be simplified as:

while (*s != '\r') {

Cc: Krzysztof Piotr Oledzki o...@ans.pl
Signed-off-by: Simon Horman ho...@verge.net.au
---
 include/common/standard.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/common/standard.h b/include/common/standard.h
index b1db821..d931925 100644
--- a/include/common/standard.h
+++ b/include/common/standard.h
@@ -411,7 +411,7 @@ unsigned int inetaddr_host_lim_ret(char *text, char *stop, 
char **ret);
 
 static inline char *cut_crlf(char *s) {
 
-   while (*s != '\r' || *s == '\n') {
+   while (*s != '\r'  *s != '\n') {
char *p = s++;
 
if (!*p)
-- 
1.7.10.4