Re: [Qemu-devel] [PATCH v4 RFC 3/3] util/uri.c: wrap single statement blocks with braces {}
I can't be too happy to know my first patch get accepted! ^_^
I hope I can make more contributions to community!
> -Original Messages-
> From: "Thomas Huth" <th...@redhat.com>
> Sent Time: 2018-02-26 14:49:46 (Monday)
> To: "Su Hang" <suhan...@mails.ucas.ac.cn>, boxa...@163.com
> Cc: qemu-devel@nongnu.org, "Stefan Hajnoczi" <stefa...@redhat.com>
> Subject: Re: [Qemu-devel] [PATCH v4 RFC 3/3] util/uri.c: wrap single
> statement blocks with braces {}
>
> On 25.02.2018 05:35, Su Hang wrote:
> > For this patch, using curly braces to wrap `if` `while` `else` statements,
> > which only hold single statement. For example:
> > '''
> > if (cond)
> > statement;
> > '''
> > to
> > '''
> > if (cond) {
> > statement;
> > }
> > '''
> >
> > And using tricks that compare the disassemblies before and after
> > code changes, to make sure code logic isn't changed:
> > '''
> > git checkout master
> > make util/uri.o
> > strip util/uri.o
> > objdump -Drx util/uri.o > /tmp/uri-master.txt
> > git checkout cleanupbranch
> > make util/uri.o
> > strip util/uri.o
> > objdump -Drx util/uri.o > /tmp/uri-cleanup.txt
> > diff -u /tmp/uri-*.txt
> > '''
> >
> > With that, all complaints raised by checkpatch.pl have been suppressed.
> >
> > Suggested-by: Thomas Huth <th...@redhat.com>
> > Suggested-by: Eric Blake <ebl...@redhat.com>
> > Signed-off-by: Su Hang <suhan...@mails.ucas.ac.cn>
> > ---
> > util/uri.c | 463
> > +++--
> > 1 file changed, 294 insertions(+), 169 deletions(-)
>
> Reviewed-by: Thomas Huth <th...@redhat.com>
>
> I've also checked again with the "objdump" trick that there are no
> differences in the generated code, and indeed, looks good now, so I
> think I can also say:
>
> Tested-by: Thomas Huth <th...@redhat.com>
>
> Thank you very much for cleaning up that huge file!
>
> Thomas
Re: [Qemu-devel] [PATCH v4 RFC 3/3] util/uri.c: wrap single statement blocks with braces {}
On 25.02.2018 05:35, Su Hang wrote:
> For this patch, using curly braces to wrap `if` `while` `else` statements,
> which only hold single statement. For example:
> '''
> if (cond)
> statement;
> '''
> to
> '''
> if (cond) {
> statement;
> }
> '''
>
> And using tricks that compare the disassemblies before and after
> code changes, to make sure code logic isn't changed:
> '''
> git checkout master
> make util/uri.o
> strip util/uri.o
> objdump -Drx util/uri.o > /tmp/uri-master.txt
> git checkout cleanupbranch
> make util/uri.o
> strip util/uri.o
> objdump -Drx util/uri.o > /tmp/uri-cleanup.txt
> diff -u /tmp/uri-*.txt
> '''
>
> With that, all complaints raised by checkpatch.pl have been suppressed.
>
> Suggested-by: Thomas Huth
> Suggested-by: Eric Blake
> Signed-off-by: Su Hang
> ---
> util/uri.c | 463
> +++--
> 1 file changed, 294 insertions(+), 169 deletions(-)
Reviewed-by: Thomas Huth
I've also checked again with the "objdump" trick that there are no
differences in the generated code, and indeed, looks good now, so I
think I can also say:
Tested-by: Thomas Huth
Thank you very much for cleaning up that huge file!
Thomas
[Qemu-devel] [PATCH v4 RFC 3/3] util/uri.c: wrap single statement blocks with braces {}
For this patch, using curly braces to wrap `if` `while` `else` statements,
which only hold single statement. For example:
'''
if (cond)
statement;
'''
to
'''
if (cond) {
statement;
}
'''
And using tricks that compare the disassemblies before and after
code changes, to make sure code logic isn't changed:
'''
git checkout master
make util/uri.o
strip util/uri.o
objdump -Drx util/uri.o > /tmp/uri-master.txt
git checkout cleanupbranch
make util/uri.o
strip util/uri.o
objdump -Drx util/uri.o > /tmp/uri-cleanup.txt
diff -u /tmp/uri-*.txt
'''
With that, all complaints raised by checkpatch.pl have been suppressed.
Suggested-by: Thomas Huth
Suggested-by: Eric Blake
Signed-off-by: Su Hang
---
util/uri.c | 463 +++--
1 file changed, 294 insertions(+), 169 deletions(-)
diff --git a/util/uri.c b/util/uri.c
index bb2576cf2190..93ecefdaaf7d 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -211,16 +211,19 @@ static int rfc3986_parse_scheme(URI *uri, const char
**str)
{
const char *cur;
-if (str == NULL)
+if (str == NULL) {
return -1;
+}
cur = *str;
-if (!ISA_ALPHA(cur))
+if (!ISA_ALPHA(cur)) {
return 2;
+}
cur++;
while (ISA_ALPHA(cur) || ISA_DIGIT(cur) || (*cur == '+') || (*cur == '-')
||
- (*cur == '.'))
+ (*cur == '.')) {
cur++;
+}
if (uri != NULL) {
g_free(uri->scheme);
uri->scheme = g_strndup(*str, cur - *str);
@@ -248,21 +251,24 @@ static int rfc3986_parse_fragment(URI *uri, const char
**str)
{
const char *cur;
-if (str == NULL)
+if (str == NULL) {
return -1;
+}
cur = *str;
while ((ISA_PCHAR(cur)) || (*cur == '/') || (*cur == '?') ||
(*cur == '[') || (*cur == ']') ||
- ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur
+ ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur {
NEXT(cur);
+}
if (uri != NULL) {
g_free(uri->fragment);
-if (uri->cleanup & 2)
+if (uri->cleanup & 2) {
uri->fragment = g_strndup(*str, cur - *str);
-else
+} else {
uri->fragment = uri_string_unescape(*str, cur - *str, NULL);
+}
}
*str = cur;
return 0;
@@ -283,14 +289,16 @@ static int rfc3986_parse_query(URI *uri, const char **str)
{
const char *cur;
-if (str == NULL)
+if (str == NULL) {
return -1;
+}
cur = *str;
while ((ISA_PCHAR(cur)) || (*cur == '/') || (*cur == '?') ||
- ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur
+ ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur {
NEXT(cur);
+}
if (uri != NULL) {
g_free(uri->query);
uri->query = g_strndup(*str, cur - *str);
@@ -351,15 +359,17 @@ static int rfc3986_parse_user_info(URI *uri, const char
**str)
cur = *str;
while (ISA_UNRESERVED(cur) || ISA_PCT_ENCODED(cur) || ISA_SUB_DELIM(cur) ||
- (*cur == ':'))
+ (*cur == ':')) {
NEXT(cur);
+}
if (*cur == '@') {
if (uri != NULL) {
g_free(uri->user);
-if (uri->cleanup & 2)
+if (uri->cleanup & 2) {
uri->user = g_strndup(*str, cur - *str);
-else
+} else {
uri->user = uri_string_unescape(*str, cur - *str, NULL);
+}
}
*str = cur;
return 0;
@@ -385,22 +395,24 @@ static int rfc3986_parse_dec_octet(const char **str)
{
const char *cur = *str;
-if (!(ISA_DIGIT(cur)))
+if (!(ISA_DIGIT(cur))) {
return 1;
-if (!ISA_DIGIT(cur + 1))
+}
+if (!ISA_DIGIT(cur + 1)) {
cur++;
-else if ((*cur != '0') && (ISA_DIGIT(cur + 1)) && (!ISA_DIGIT(cur + 2)))
+} else if ((*cur != '0') && (ISA_DIGIT(cur + 1)) && (!ISA_DIGIT(cur + 2)))
{
cur += 2;
-else if ((*cur == '1') && (ISA_DIGIT(cur + 1)) && (ISA_DIGIT(cur + 2)))
+} else if ((*cur == '1') && (ISA_DIGIT(cur + 1)) && (ISA_DIGIT(cur + 2))) {
cur += 3;
-else if ((*cur == '2') && (*(cur + 1) >= '0') && (*(cur + 1) <= '4') &&
- (ISA_DIGIT(cur + 2)))
+} else if ((*cur == '2') && (*(cur + 1) >= '0') && (*(cur + 1) <= '4') &&
+ (ISA_DIGIT(cur + 2))) {
cur += 3;
-else if ((*cur == '2') && (*(cur + 1) == '5') && (*(cur + 2) >= '0') &&
- (*(cur + 1) <= '5'))
+} else if ((*cur == '2') && (*(cur + 1) == '5') && (*(cur + 2) >= '0') &&
+ (*(cur + 1) <= '5')) {
cur += 3;
-else
+} else {
return 1;
+}
*str = cur;
return 0;
}
@@ -430,10 +442,12 @@ static int rfc3986_parse_host(URI *uri, const char **str)
*/
if (*cur == '[') {
cur++;
-while ((*cur != ']') && (*cur != 0))
+
[Qemu-devel] [PATCH v4 RFC 3/3] util/uri.c: wrap single statement blocks with braces {}
For this patch, using curly braces to wrap `if` `while` `else` statements,
which only hold single statement. For example:
'''
if (cond)
statement;
'''
to
'''
if (cond) {
statement;
}
'''
And using tricks that compare the disassemblies before and after
code changes, to make sure code logic isn't changed:
'''
git checkout master
make util/uri.o
strip util/uri.o
objdump -Drx util/uri.o > /tmp/uri-master.txt
git checkout cleanupbranch
make util/uri.o
strip util/uri.o
objdump -Drx util/uri.o > /tmp/uri-cleanup.txt
diff -u /tmp/uri-*.txt
'''
With that, all complaints raised by checkpatch.pl have been suppressed.
Suggested-by: Thomas Huth
Suggested-by: Eric Blake
Signed-off-by: Su Hang
---
util/uri.c | 463 +++--
1 file changed, 294 insertions(+), 169 deletions(-)
diff --git a/util/uri.c b/util/uri.c
index bb2576cf2190..93ecefdaaf7d 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -211,16 +211,19 @@ static int rfc3986_parse_scheme(URI *uri, const char
**str)
{
const char *cur;
-if (str == NULL)
+if (str == NULL) {
return -1;
+}
cur = *str;
-if (!ISA_ALPHA(cur))
+if (!ISA_ALPHA(cur)) {
return 2;
+}
cur++;
while (ISA_ALPHA(cur) || ISA_DIGIT(cur) || (*cur == '+') || (*cur == '-')
||
- (*cur == '.'))
+ (*cur == '.')) {
cur++;
+}
if (uri != NULL) {
g_free(uri->scheme);
uri->scheme = g_strndup(*str, cur - *str);
@@ -248,21 +251,24 @@ static int rfc3986_parse_fragment(URI *uri, const char
**str)
{
const char *cur;
-if (str == NULL)
+if (str == NULL) {
return -1;
+}
cur = *str;
while ((ISA_PCHAR(cur)) || (*cur == '/') || (*cur == '?') ||
(*cur == '[') || (*cur == ']') ||
- ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur
+ ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur {
NEXT(cur);
+}
if (uri != NULL) {
g_free(uri->fragment);
-if (uri->cleanup & 2)
+if (uri->cleanup & 2) {
uri->fragment = g_strndup(*str, cur - *str);
-else
+} else {
uri->fragment = uri_string_unescape(*str, cur - *str, NULL);
+}
}
*str = cur;
return 0;
@@ -283,14 +289,16 @@ static int rfc3986_parse_query(URI *uri, const char **str)
{
const char *cur;
-if (str == NULL)
+if (str == NULL) {
return -1;
+}
cur = *str;
while ((ISA_PCHAR(cur)) || (*cur == '/') || (*cur == '?') ||
- ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur
+ ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur {
NEXT(cur);
+}
if (uri != NULL) {
g_free(uri->query);
uri->query = g_strndup(*str, cur - *str);
@@ -351,15 +359,17 @@ static int rfc3986_parse_user_info(URI *uri, const char
**str)
cur = *str;
while (ISA_UNRESERVED(cur) || ISA_PCT_ENCODED(cur) || ISA_SUB_DELIM(cur) ||
- (*cur == ':'))
+ (*cur == ':')) {
NEXT(cur);
+}
if (*cur == '@') {
if (uri != NULL) {
g_free(uri->user);
-if (uri->cleanup & 2)
+if (uri->cleanup & 2) {
uri->user = g_strndup(*str, cur - *str);
-else
+} else {
uri->user = uri_string_unescape(*str, cur - *str, NULL);
+}
}
*str = cur;
return 0;
@@ -385,22 +395,24 @@ static int rfc3986_parse_dec_octet(const char **str)
{
const char *cur = *str;
-if (!(ISA_DIGIT(cur)))
+if (!(ISA_DIGIT(cur))) {
return 1;
-if (!ISA_DIGIT(cur + 1))
+}
+if (!ISA_DIGIT(cur + 1)) {
cur++;
-else if ((*cur != '0') && (ISA_DIGIT(cur + 1)) && (!ISA_DIGIT(cur + 2)))
+} else if ((*cur != '0') && (ISA_DIGIT(cur + 1)) && (!ISA_DIGIT(cur + 2)))
{
cur += 2;
-else if ((*cur == '1') && (ISA_DIGIT(cur + 1)) && (ISA_DIGIT(cur + 2)))
+} else if ((*cur == '1') && (ISA_DIGIT(cur + 1)) && (ISA_DIGIT(cur + 2))) {
cur += 3;
-else if ((*cur == '2') && (*(cur + 1) >= '0') && (*(cur + 1) <= '4') &&
- (ISA_DIGIT(cur + 2)))
+} else if ((*cur == '2') && (*(cur + 1) >= '0') && (*(cur + 1) <= '4') &&
+ (ISA_DIGIT(cur + 2))) {
cur += 3;
-else if ((*cur == '2') && (*(cur + 1) == '5') && (*(cur + 2) >= '0') &&
- (*(cur + 1) <= '5'))
+} else if ((*cur == '2') && (*(cur + 1) == '5') && (*(cur + 2) >= '0') &&
+ (*(cur + 1) <= '5')) {
cur += 3;
-else
+} else {
return 1;
+}
*str = cur;
return 0;
}
@@ -430,10 +442,12 @@ static int rfc3986_parse_host(URI *uri, const char **str)
*/
if (*cur == '[') {
cur++;
-while ((*cur != ']') && (*cur != 0))
+
