From: Luiz A. v. D. <lui...@gm...> - 2011-05-25 09:21:34
|
From: Luiz Augusto von Dentz <lui...@in...> make[3]: Entering directory `/home/vudentz/git/openobex/apps/obex_test' CC obex_test_client.o obex_test_client.c: In function ‘push_client’: obex_test_client.c:229:6: error: variable ‘num’ set but not used obex_test_client.c: In function ‘put_client’: obex_test_client.c:290:6: error: variable ‘num’ set but not used obex_test_client.c: In function ‘get_client’: obex_test_client.c:347:6: error: variable ‘num’ set but not used obex_test_client.c: In function ‘setpath_client’: obex_test_client.c:418:6: error: variable ‘num’ set but not used cc1: all warnings being treated as errors --- apps/obex_test/obex_test_client.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/apps/obex_test/obex_test_client.c b/apps/obex_test/obex_test_client.c index 0bf5d42..fc54e20 100644 --- a/apps/obex_test/obex_test_client.c +++ b/apps/obex_test/obex_test_client.c @@ -235,6 +235,11 @@ void push_client(obex_t *handle) printf("PUSH filename> "); num = scanf("%s", fname); + if (num != 1) { + perror("scanf"); + return; + } + bfname = strdup(basename(fname)); buf = easy_readfile(fname, &file_size); @@ -294,6 +299,11 @@ void put_client(obex_t *handle) printf("PUT file (local, remote)> "); num = scanf("%s %s", lname, rname); + if (num != 2) { + perror("scanf:"); + return; + } + buf = easy_readfile(lname, &file_size); if(buf == NULL) { @@ -349,6 +359,10 @@ void get_client(obex_t *handle, struct context *gt) printf("GET File> "); num = scanf("%s", req_name); + if (num != 1) { + perror("scanf:"); + return; + } if(! (object = OBEX_ObjectNew(handle, OBEX_CMD_GET))) { printf("Error\n"); @@ -420,6 +434,10 @@ void setpath_client(obex_t *handle) printf("SETPATH> "); num = scanf("%s", path); + if (num != 1) { + perror("scanf:"); + return; + } if(! (object = OBEX_ObjectNew(handle, OBEX_CMD_SETPATH))) { printf("Error\n"); -- 1.7.5.1 |
From: Luiz A. v. D. <lui...@gm...> - 2011-06-02 14:44:35
|
From: Luiz Augusto von Dentz <lui...@in...> obex_test_client.c: In function ‘push_client’: obex_test_client.c:229:6: error: variable ‘num’ set but not used obex_test_client.c: In function ‘put_client’: obex_test_client.c:290:6: error: variable ‘num’ set but not used obex_test_client.c: In function ‘get_client’: obex_test_client.c:347:6: error: variable ‘num’ set but not used obex_test_client.c: In function ‘setpath_client’: obex_test_client.c:418:6: error: variable ‘num’ set but not used cc1: all warnings being treated as errors --- apps/obex_test/obex_test_client.c | 31 +++++++++++++++++++++++++++---- 1 files changed, 27 insertions(+), 4 deletions(-) diff --git a/apps/obex_test/obex_test_client.c b/apps/obex_test/obex_test_client.c index 0bf5d42..739f04a 100644 --- a/apps/obex_test/obex_test_client.c +++ b/apps/obex_test/obex_test_client.c @@ -234,7 +234,13 @@ void push_client(obex_t *handle) int file_size; printf("PUSH filename> "); - num = scanf("%s", fname); + memset(fname, 0, sizeof(fname)); + num = scanf("%199c", fname); + if (num != 1) { + perror("scanf"); + return; + } + bfname = strdup(basename(fname)); buf = easy_readfile(fname, &file_size); @@ -293,7 +299,14 @@ void put_client(obex_t *handle) int file_size; printf("PUT file (local, remote)> "); - num = scanf("%s %s", lname, rname); + memset(lname, 0, sizeof(lname)); + memset(rname, 0, sizeof(rname)); + num = scanf("%199c %199c", lname, rname); + if (num != 2) { + perror("scanf:"); + return; + } + buf = easy_readfile(lname, &file_size); if(buf == NULL) { @@ -348,7 +361,12 @@ void get_client(obex_t *handle, struct context *gt) obex_headerdata_t hd; printf("GET File> "); - num = scanf("%s", req_name); + memset(req_name, 0, sizeof(req_name)); + num = scanf("%199c", req_name); + if (num != 1) { + perror("scanf:"); + return; + } if(! (object = OBEX_ObjectNew(handle, OBEX_CMD_GET))) { printf("Error\n"); @@ -419,7 +437,12 @@ void setpath_client(obex_t *handle) obex_headerdata_t hd; printf("SETPATH> "); - num = scanf("%s", path); + memset(path, 0, sizeof(path)); + num = scanf("%199c", path); + if (num != 1) { + perror("scanf:"); + return; + } if(! (object = OBEX_ObjectNew(handle, OBEX_CMD_SETPATH))) { printf("Error\n"); -- 1.7.5.2 |
From: Luiz A. v. D. <lui...@gm...> - 2011-06-02 14:44:39
|
From: Luiz Augusto von Dentz <lui...@in...> ircp_server.c: In function ‘ircp_srv_receive’: ircp_server.c:303:1: error: ISO C90 forbids mixed declarations and code ircp_server.c: In function ‘ircp_srv_recv’: ircp_server.c:346:6: error: variable ‘err’ set but not used ircp_server.c: In function ‘ircp_srv_receive’: ircp_server.c:369:1: error: expected declaration or statement at end of input ircp_server.c:369:1: error: control reaches end of non-void function cc1: all warnings being treated as errors --- apps/ircp/ircp_client.c | 5 +++++ apps/ircp/ircp_server.c | 11 +++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/apps/ircp/ircp_client.c b/apps/ircp/ircp_client.c index 7f10d0c..1acd4a8 100644 --- a/apps/ircp/ircp_client.c +++ b/apps/ircp/ircp_client.c @@ -395,6 +395,11 @@ int ircp_put(ircp_client_t *cli, char *name) char *dirname; err = chdir(name); + if (err < 0) { + perror("chdir:"); + return -1; + } + name = "."; /* Get real name of new wd, extract last part of and do setpath to it */ diff --git a/apps/ircp/ircp_server.c b/apps/ircp/ircp_server.c index 08de942..749a84e 100644 --- a/apps/ircp/ircp_server.c +++ b/apps/ircp/ircp_server.c @@ -285,8 +285,13 @@ int ircp_srv_receive(ircp_server_t *srv, obex_object_t *object, int finished) srv->infocb(IRCP_EV_OK, ""); } else { - if(srv->fd > 0) + if(srv->fd > 0) { len = write(srv->fd, body, body_len); + if (len < 0) { + perror("write:"); + return -1; + } + } } return 1; } @@ -360,6 +365,8 @@ int ircp_srv_recv(ircp_server_t *srv, char *inbox) /* Go back to inbox */ err = chdir(inbox); - + if (err < 0) + perror("chdir:"); + return ret; } -- 1.7.5.2 |
From: Luiz A. v. D. <lui...@gm...> - 2011-05-25 09:21:35
|
From: Luiz Augusto von Dentz <lui...@in...> make[3]: Entering directory `/home/vudentz/git/openobex/apps/ircp' CC ircp_server.o CC ircp_client.o ircp_server.c: In function ‘ircp_srv_receive’: ircp_server.c:303:1: error: ISO C90 forbids mixed declarations and code ircp_server.c: In function ‘ircp_srv_recv’: ircp_server.c:346:6: error: variable ‘err’ set but not used ircp_server.c: In function ‘ircp_srv_receive’: ircp_server.c:369:1: error: expected declaration or statement at end of input ircp_server.c:369:1: error: control reaches end of non-void function cc1: all warnings being treated as errors --- apps/ircp/ircp_client.c | 5 +++++ apps/ircp/ircp_server.c | 11 +++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/apps/ircp/ircp_client.c b/apps/ircp/ircp_client.c index 7f10d0c..1acd4a8 100644 --- a/apps/ircp/ircp_client.c +++ b/apps/ircp/ircp_client.c @@ -395,6 +395,11 @@ int ircp_put(ircp_client_t *cli, char *name) char *dirname; err = chdir(name); + if (err < 0) { + perror("chdir:"); + return -1; + } + name = "."; /* Get real name of new wd, extract last part of and do setpath to it */ diff --git a/apps/ircp/ircp_server.c b/apps/ircp/ircp_server.c index 08de942..749a84e 100644 --- a/apps/ircp/ircp_server.c +++ b/apps/ircp/ircp_server.c @@ -285,8 +285,13 @@ int ircp_srv_receive(ircp_server_t *srv, obex_object_t *object, int finished) srv->infocb(IRCP_EV_OK, ""); } else { - if(srv->fd > 0) + if(srv->fd > 0) { len = write(srv->fd, body, body_len); + if (len < 0) { + perror("write:"); + return -1; + } + } } return 1; } @@ -360,6 +365,8 @@ int ircp_srv_recv(ircp_server_t *srv, char *inbox) /* Go back to inbox */ err = chdir(inbox); - + if (err < 0) + perror("chdir:"); + return ret; } -- 1.7.5.1 |
From: Hendrik S. <po...@he...> - 2011-05-25 09:34:03
|
Zitat von Luiz Augusto von Dentz <lui...@gm...>: [...] Both of those are already fixed in my releasing-2.0 tree, see https://gitorious.org/openobex/mainline/commit/a862b3d3eec7c0008ebcfcf8a75150f8a3f3bc19 I already posted them here. Except one mail from Johan, nothing happened. I admit that you did clean up ircp better than I did, though :) Do you want to rebase the second patch on my tree, basically on-top of https://gitorious.org/openobex/mainline/commits/ebc1dc31734bec3a2d8b172cf959b3844a29a3d9 Note that those are build warnings, not build errors (except when using maintainer mode). HS |
From: Luiz A. v. D. <lui...@gm...> - 2011-05-27 07:54:20
|
Hi Hendrik, On Wed, May 25, 2011 at 12:33 PM, Hendrik Sattler <po...@he...> wrote: > Zitat von Luiz Augusto von Dentz <lui...@gm...>: > [...] > > Both of those are already fixed in my releasing-2.0 tree, see > https://gitorious.org/openobex/mainline/commit/a862b3d3eec7c0008ebcfcf8a75150f8a3f3bc19 > > I already posted them here. Except one mail from Johan, nothing happened. I > admit that you did clean up ircp better than I did, though :) > Do you want to rebase the second patch on my tree, basically on-top of > https://gitorious.org/openobex/mainline/commits/ebc1dc31734bec3a2d8b172cf959b3844a29a3d9 Well in that case why you don't rebase your tree, my patches seems to fix just the issues with GCC 4.6 while yours fixes some other things so it might be clearer to just have them split meaning that you should rework your patches anyway. I know this sound inconvenient because you have posted similar changes before, but they are not upstream, besides that if I do rebase we will have a commit to fix another commit not upstream (e.g. handling scanf return correctly), which IMO is not good for sake of repository history. This fixes should go in as soon as possible. > Note that those are build warnings, not build errors (except when using > maintainer mode). Which is the whole point to force fixing and not just ignore them. -- Luiz Augusto von Dentz Computer Engineer |
From: Hendrik S. <po...@he...> - 2011-05-27 11:00:53
|
Am Freitag, 27. Mai 2011, 09:54:13 schrieb Luiz Augusto von Dentz: > Hi Hendrik, > > On Wed, May 25, 2011 at 12:33 PM, Hendrik Sattler > > <po...@he...> wrote: > > Zitat von Luiz Augusto von Dentz <lui...@gm...>: > > [...] > > > > Both of those are already fixed in my releasing-2.0 tree, see > > https://gitorious.org/openobex/mainline/commit/a862b3d3eec7c0008ebcfcf8a7 > > 5150f8a3f3bc19 > > > > I already posted them here. Except one mail from Johan, nothing happened. > > I admit that you did clean up ircp better than I did, though :) > > Do you want to rebase the second patch on my tree, basically on-top of > > https://gitorious.org/openobex/mainline/commits/ebc1dc31734bec3a2d8b172cf > > 959b3844a29a3d9 > > Well in that case why you don't rebase your tree, my patches seems to > fix just the issues with GCC 4.6 while yours fixes some other things > so it might be clearer to just have them split meaning that you should > rework your patches anyway. I know this sound inconvenient because you > have posted similar changes before, but they are not upstream, besides > that if I do rebase we will have a commit to fix another commit not > upstream (e.g. handling scanf return correctly), which IMO is not good > for sake of repository history. This fixes should go in as soon as > possible. I'm sick of the current situation, so I don't care anymore. HS |
From: Hendrik S. <po...@he...> - 2011-05-27 11:04:57
|
Am Freitag, 27. Mai 2011, 09:54:13 schrieb Luiz Augusto von Dentz: > > Note that those are build warnings, not build errors (except when using > > maintainer mode). > > Which is the whole point to force fixing and not just ignore them. Your commit message is wrong, that's what I meant! HS |
From: Luiz A. v. D. <lui...@gm...> - 2011-06-01 10:32:25
|
Hi Hendrik, On Fri, May 27, 2011 at 2:04 PM, Hendrik Sattler <po...@he...> wrote: > Am Freitag, 27. Mai 2011, 09:54:13 schrieb Luiz Augusto von Dentz: >> > Note that those are build warnings, not build errors (except when using >> > maintainer mode). >> >> Which is the whole point to force fixing and not just ignore them. > > Your commit message is wrong, that's what I meant! I did rebase your changes on top of my build fixes here: git://gitorious.org/~vudentz/openobex/vudentz-clone.git If you are ok, I will resend my 2 build fixes again. I also did a quick review on the other changes and noticed at least one use of c++ comment style '//' and missing note about action command support on ChangeLog, can you fix those? And please lets be more careful with codestyle, there are some files with space mixed with tabs and a lot of empty likes with tabs. If those issues are fixed we can do 2.0 release. -- Luiz Augusto von Dentz Computer Engineer |
From: Hendrik S. <po...@he...> - 2011-06-01 11:11:29
|
Zitat von Luiz Augusto von Dentz <lui...@gm...>: > Hi Hendrik, > > On Fri, May 27, 2011 at 2:04 PM, Hendrik Sattler > <po...@he...> wrote: >> Am Freitag, 27. Mai 2011, 09:54:13 schrieb Luiz Augusto von Dentz: >>> > Note that those are build warnings, not build errors (except when using >>> > maintainer mode). >>> >>> Which is the whole point to force fixing and not just ignore them. >> >> Your commit message is wrong, that's what I meant! > > I did rebase your changes on top of my build fixes here: > git://gitorious.org/~vudentz/openobex/vudentz-clone.git > > If you are ok, I will resend my 2 build fixes again. I also did a > quick review on the other changes and noticed at least one use of c++ > comment style '//' Citing from the ISO 9899-1999 (C99) standard: "6.4.9 Comments 1 Except within a character constant, a string literal, or a comment, the characters /* introduce a comment. The contents of such a comment are examined only to identify multibyte characters and to find the characters */ that terminate it. 2 Except within a character constant, a string literal, or a comment, the characters // introduce a comment that includes all multibyte characters up to, but not including, the next new-line character. The contents of such a comment are examined only to identify multibyte characters and to find the terminating new-line character." It's the year 2011, so using /* some comment */ or // some comment are both absolutely valid C. Johan also mentioned this and I gave him the same response. > and missing note about action command support on > ChangeLog, can you fix those? Yes. > And please lets be more careful with > codestyle, there are some files with space mixed with tabs and a lot > of empty likes with tabs. I'll check that. The long week-end in Germany should give me enough time :) > If those issues are fixed we can do 2.0 release. Where? HS |
From: Luiz A. v. D. <lui...@gm...> - 2011-06-02 14:42:20
|
Hi Hendrik, On Wed, Jun 1, 2011 at 2:11 PM, Hendrik Sattler <po...@he...> wrote: > Zitat von Luiz Augusto von Dentz <lui...@gm...>: > >> Hi Hendrik, >> >> On Fri, May 27, 2011 at 2:04 PM, Hendrik Sattler >> <po...@he...> wrote: >>> Am Freitag, 27. Mai 2011, 09:54:13 schrieb Luiz Augusto von Dentz: >>>> > Note that those are build warnings, not build errors (except when using >>>> > maintainer mode). >>>> >>>> Which is the whole point to force fixing and not just ignore them. >>> >>> Your commit message is wrong, that's what I meant! >> >> I did rebase your changes on top of my build fixes here: >> git://gitorious.org/~vudentz/openobex/vudentz-clone.git >> >> If you are ok, I will resend my 2 build fixes again. I also did a >> quick review on the other changes and noticed at least one use of c++ >> comment style '//' > > Citing from the ISO 9899-1999 (C99) standard: > "6.4.9 Comments > 1 Except within a character constant, a string literal, or a comment, > the characters /* introduce a comment. The contents of such a comment > are examined only to identify multibyte characters and to find the > characters */ that terminate it. > > 2 Except within a character constant, a string literal, or a comment, > the characters // introduce a comment that includes all multibyte > characters up to, but not including, the next new-line character. The > contents of such a comment are examined only to identify multibyte > characters and to find the terminating new-line character." > > It's the year 2011, so using > /* some comment */ > or > // some comment > are both absolutely valid C. Johan also mentioned this and I gave him > the same response. You are right that it is in fact a valid comment format as per C99, but you should have know that we mostly follow linux kernel code style (http://www.kernel.org/doc/Documentation/CodingStyle), which say: Linux style for comments is the C89 "/* ... */" style. Don't use C99-style "// ..." comments. Coding style is a matter of personal preference, please lets not start arguing about it now, besides you didn't really responded if the patches do look ok to you so I will assume you are and will resent them in a moment so we don't waste more time with this. >> and missing note about action command support on >> ChangeLog, can you fix those? > > Yes. > >> And please lets be more careful with >> codestyle, there are some files with space mixed with tabs and a lot >> of empty likes with tabs. > > I'll check that. The long week-end in Germany should give me enough time :) > >> If those issues are fixed we can do 2.0 release. > > Where? In your tree, also please send them as a proper git patch (git format-path + git send-email) so we got them in the mail archive properly, if the series is too big e.g 10 > commits if possible break them in sub series waiting each series to be properly applied upstream before sending the next. I will try to help Johan to review your changes like sometimes I do for obexd. -- Luiz Augusto von Dentz Computer Engineer |
From: Luiz A. v. D. <lui...@gm...> - 2011-06-03 07:24:28
|
Hi Hendrik, On Thu, Jun 2, 2011 at 5:42 PM, Luiz Augusto von Dentz <lui...@gm...> wrote: > Hi Hendrik, > > On Wed, Jun 1, 2011 at 2:11 PM, Hendrik Sattler <po...@he...> wrote: >> Zitat von Luiz Augusto von Dentz <lui...@gm...>: >> >>> Hi Hendrik, >>> >>> On Fri, May 27, 2011 at 2:04 PM, Hendrik Sattler >>> <po...@he...> wrote: >>>> Am Freitag, 27. Mai 2011, 09:54:13 schrieb Luiz Augusto von Dentz: >>>>> > Note that those are build warnings, not build errors (except when using >>>>> > maintainer mode). >>>>> >>>>> Which is the whole point to force fixing and not just ignore them. >>>> >>>> Your commit message is wrong, that's what I meant! >>> >>> I did rebase your changes on top of my build fixes here: >>> git://gitorious.org/~vudentz/openobex/vudentz-clone.git >>> >>> If you are ok, I will resend my 2 build fixes again. I also did a >>> quick review on the other changes and noticed at least one use of c++ >>> comment style '//' >> >> Citing from the ISO 9899-1999 (C99) standard: >> "6.4.9 Comments >> 1 Except within a character constant, a string literal, or a comment, >> the characters /* introduce a comment. The contents of such a comment >> are examined only to identify multibyte characters and to find the >> characters */ that terminate it. >> >> 2 Except within a character constant, a string literal, or a comment, >> the characters // introduce a comment that includes all multibyte >> characters up to, but not including, the next new-line character. The >> contents of such a comment are examined only to identify multibyte >> characters and to find the terminating new-line character." >> >> It's the year 2011, so using >> /* some comment */ >> or >> // some comment >> are both absolutely valid C. Johan also mentioned this and I gave him >> the same response. > > You are right that it is in fact a valid comment format as per C99, > but you should have know that we mostly follow linux kernel code style > (http://www.kernel.org/doc/Documentation/CodingStyle), which say: > > Linux style for comments is the C89 "/* ... */" style. > Don't use C99-style "// ..." comments. > > Coding style is a matter of personal preference, please lets not start > arguing about it now, besides you didn't really responded if the > patches do look ok to you so I will assume you are and will resent > them in a moment so we don't waste more time with this. > >>> and missing note about action command support on >>> ChangeLog, can you fix those? >> >> Yes. >> >>> And please lets be more careful with >>> codestyle, there are some files with space mixed with tabs and a lot >>> of empty likes with tabs. >> >> I'll check that. The long week-end in Germany should give me enough time :) >> >>> If those issues are fixed we can do 2.0 release. >> >> Where? > > In your tree, also please send them as a proper git patch (git > format-path + git send-email) so we got them in the mail archive > properly, if the series is too big e.g 10 > commits if possible break > them in sub series waiting each series to be properly applied upstream > before sending the next. I will try to help Johan to review your > changes like sometimes I do for obexd. In case you meant where we will do the release, I was talking about tag 2.0 upstream on git.kernel.org repository and create the tarball as usual. -- Luiz Augusto von Dentz Computer Engineer |
From: Hendrik S. <po...@he...> - 2011-06-05 08:55:27
|
Am Freitag, 3. Juni 2011, 09:24:21 schrieb Luiz Augusto von Dentz: > Hi Hendrik, > > On Thu, Jun 2, 2011 at 5:42 PM, Luiz Augusto von Dentz > > <lui...@gm...> wrote: > > Hi Hendrik, > > > > On Wed, Jun 1, 2011 at 2:11 PM, Hendrik Sattler <po...@he...> wrote: > >> Zitat von Luiz Augusto von Dentz <lui...@gm...>: > >>> Hi Hendrik, > >>> > >>> On Fri, May 27, 2011 at 2:04 PM, Hendrik Sattler > >>> > >>> <po...@he...> wrote: > >>>> Am Freitag, 27. Mai 2011, 09:54:13 schrieb Luiz Augusto von Dentz: > >>>>> > Note that those are build warnings, not build errors (except when > >>>>> > using maintainer mode). > >>>>> > >>>>> Which is the whole point to force fixing and not just ignore them. > >>>> > >>>> Your commit message is wrong, that's what I meant! > >>> > >>> I did rebase your changes on top of my build fixes here: > >>> git://gitorious.org/~vudentz/openobex/vudentz-clone.git > >>> > >>> If you are ok, I will resend my 2 build fixes again. I also did a > >>> quick review on the other changes and noticed at least one use of c++ > >>> comment style '//' > >> > >> Citing from the ISO 9899-1999 (C99) standard: > >> "6.4.9 Comments > >> 1 Except within a character constant, a string literal, or a comment, > >> the characters /* introduce a comment. The contents of such a comment > >> are examined only to identify multibyte characters and to find the > >> characters */ that terminate it. > >> > >> 2 Except within a character constant, a string literal, or a comment, > >> the characters // introduce a comment that includes all multibyte > >> characters up to, but not including, the next new-line character. The > >> contents of such a comment are examined only to identify multibyte > >> characters and to find the terminating new-line character." > >> > >> It's the year 2011, so using > >> /* some comment */ > >> or > >> // some comment > >> are both absolutely valid C. Johan also mentioned this and I gave him > >> the same response. > > > > You are right that it is in fact a valid comment format as per C99, > > but you should have know that we mostly follow linux kernel code style > > (http://www.kernel.org/doc/Documentation/CodingStyle), which say: > > > > Linux style for comments is the C89 "/* ... */" style. > > Don't use C99-style "// ..." comments. > > > > Coding style is a matter of personal preference, please lets not start > > arguing about it now, besides you didn't really responded if the > > patches do look ok to you so I will assume you are and will resent > > them in a moment so we don't waste more time with this. "linux kernel code style" has lots of non-sense beyond reasoning, not just comments. It is written nowhere that this style is used with OpenOBEX, neither was it discussed ever. And yes, I reject this coding style in many parts. > >>> and missing note about action command support on > >>> ChangeLog, can you fix those? > >> > >> Yes. I _was_ already mentioned but I changed it: - Add new definitions from OBEX 1.3 errata + Add new definitions from OBEX 1.3 errata: ACTION command and headers > >>> And please lets be more careful with > >>> codestyle, there are some files with space mixed with tabs and a lot > >>> of empty likes with tabs. > >> > >> I'll check that. The long week-end in Germany should give me enough time > >> :) > >> > >>> If those issues are fixed we can do 2.0 release. > >> > >> Where? > > > > In your tree, also please send them as a proper git patch (git > > format-path + git send-email) so we got them in the mail archive > > properly, if the series is too big e.g 10 > commits if possible break > > them in sub series waiting each series to be properly applied upstream > > before sending the next. I will try to help Johan to review your > > changes like sometimes I do for obexd. > > In case you meant where we will do the release, I was talking about > tag 2.0 upstream on git.kernel.org repository and create the tarball > as usual. Hint: "git send-email" itself is enough. I changed the patch you edited and reordered the patch set. I will send them. For the "trailing spaces" issue: how do I tell git to give me a warning for a _present_ patch? HS |
From: Luiz A. v. D. <lui...@gm...> - 2011-06-06 04:46:19
|
Hi Hendrik, On Sun, Jun 5, 2011 at 11:55 AM, Hendrik Sattler <po...@he...> wrote: > Am Freitag, 3. Juni 2011, 09:24:21 schrieb Luiz Augusto von Dentz: >> Hi Hendrik, >> >> On Thu, Jun 2, 2011 at 5:42 PM, Luiz Augusto von Dentz >> >> <lui...@gm...> wrote: >> > Hi Hendrik, >> > >> > On Wed, Jun 1, 2011 at 2:11 PM, Hendrik Sattler <po...@he...> > wrote: >> >> Zitat von Luiz Augusto von Dentz <lui...@gm...>: >> >>> Hi Hendrik, >> >>> >> >>> On Fri, May 27, 2011 at 2:04 PM, Hendrik Sattler >> >>> >> >>> <po...@he...> wrote: >> >>>> Am Freitag, 27. Mai 2011, 09:54:13 schrieb Luiz Augusto von Dentz: >> >>>>> > Note that those are build warnings, not build errors (except when >> >>>>> > using maintainer mode). >> >>>>> >> >>>>> Which is the whole point to force fixing and not just ignore them. >> >>>> >> >>>> Your commit message is wrong, that's what I meant! >> >>> >> >>> I did rebase your changes on top of my build fixes here: >> >>> git://gitorious.org/~vudentz/openobex/vudentz-clone.git >> >>> >> >>> If you are ok, I will resend my 2 build fixes again. I also did a >> >>> quick review on the other changes and noticed at least one use of c++ >> >>> comment style '//' >> >> >> >> Citing from the ISO 9899-1999 (C99) standard: >> >> "6.4.9 Comments >> >> 1 Except within a character constant, a string literal, or a comment, >> >> the characters /* introduce a comment. The contents of such a comment >> >> are examined only to identify multibyte characters and to find the >> >> characters */ that terminate it. >> >> >> >> 2 Except within a character constant, a string literal, or a comment, >> >> the characters // introduce a comment that includes all multibyte >> >> characters up to, but not including, the next new-line character. The >> >> contents of such a comment are examined only to identify multibyte >> >> characters and to find the terminating new-line character." >> >> >> >> It's the year 2011, so using >> >> /* some comment */ >> >> or >> >> // some comment >> >> are both absolutely valid C. Johan also mentioned this and I gave him >> >> the same response. >> > >> > You are right that it is in fact a valid comment format as per C99, >> > but you should have know that we mostly follow linux kernel code style >> > (http://www.kernel.org/doc/Documentation/CodingStyle), which say: >> > >> > Linux style for comments is the C89 "/* ... */" style. >> > Don't use C99-style "// ..." comments. >> > >> > Coding style is a matter of personal preference, please lets not start >> > arguing about it now, besides you didn't really responded if the >> > patches do look ok to you so I will assume you are and will resent >> > them in a moment so we don't waste more time with this. > > "linux kernel code style" has lots of non-sense beyond reasoning, not just > comments. It is written nowhere that this style is used with OpenOBEX, neither > was it discussed ever. And yes, I reject this coding style in many parts. Well I thought OpenOBEX follows BlueZ in this... anyway we should be following the some coding style, if it is not defined we should talk to Johan and get this sort out. >> >>> and missing note about action command support on >> >>> ChangeLog, can you fix those? >> >> >> >> Yes. > > I _was_ already mentioned but I changed it: > - Add new definitions from OBEX 1.3 errata > + Add new definitions from OBEX 1.3 errata: ACTION command and headers Looks good to me. >> >>> And please lets be more careful with >> >>> codestyle, there are some files with space mixed with tabs and a lot >> >>> of empty likes with tabs. >> >> >> >> I'll check that. The long week-end in Germany should give me enough time >> >> :) >> >> >> >>> If those issues are fixed we can do 2.0 release. >> >> >> >> Where? >> > >> > In your tree, also please send them as a proper git patch (git >> > format-path + git send-email) so we got them in the mail archive >> > properly, if the series is too big e.g 10 > commits if possible break >> > them in sub series waiting each series to be properly applied upstream >> > before sending the next. I will try to help Johan to review your >> > changes like sometimes I do for obexd. >> >> In case you meant where we will do the release, I was talking about >> tag 2.0 upstream on git.kernel.org repository and create the tarball >> as usual. > > Hint: "git send-email" itself is enough. > > I changed the patch you edited and reordered the patch set. I will send them. > For the "trailing spaces" issue: how do I tell git to give me a warning for a > _present_ patch? No idea, I use vi with some extra checks for trailing spaces and things like that, but Im quite sure you can do this with git. Btw, your new series looks good enough to be applied. -- Luiz Augusto von Dentz Computer Engineer |
From: Hendrik S. <po...@he...> - 2011-06-06 08:46:44
|
Zitat von Luiz Augusto von Dentz <lui...@gm...>: > Btw, your new series looks good enough to be applied. Great. I've split it into three series and will send the next series if the previous one is in git. You can selectivly (git reset ....) pull those patches from my "releasing-2.0" branch at gitorious. HS |
From: Hendrik S. <po...@he...> - 2011-06-06 08:50:20
|
Zitat von Hendrik Sattler <po...@he...>: > Zitat von Luiz Augusto von Dentz <lui...@gm...>: >> Btw, your new series looks good enough to be applied. > > Great. I've split it into three series and will send the next series > if the previous one is in git. > > You can selectivly (git reset ....) pull those patches from my > "releasing-2.0" branch at gitorious. Or at least this evening it will :) The current one is not fully up-to-date. HS |
From: Hendrik S. <po...@he...> - 2011-06-06 20:32:50
|
Am Montag, 6. Juni 2011, 10:50:08 schrieb Hendrik Sattler: > Zitat von Hendrik Sattler <po...@he...>: > > Zitat von Luiz Augusto von Dentz <lui...@gm...>: > >> Btw, your new series looks good enough to be applied. > > > > Great. I've split it into three series and will send the next series > > if the previous one is in git. > > > > You can selectivly (git reset ....) pull those patches from my > > "releasing-2.0" branch at gitorious. > > Or at least this evening it will :) > The current one is not fully up-to-date. Oh no, it was. Just the dumb corporate proxy :-( I feel like talking to myself... oh wait... I do ;-) HS |
From: Hendrik S. <po...@he...> - 2011-06-06 20:50:21
|
Am Montag, 6. Juni 2011, 22:32:02 schrieb Hendrik Sattler: > Am Montag, 6. Juni 2011, 10:50:08 schrieb Hendrik Sattler: > > Zitat von Hendrik Sattler <po...@he...>: > > > Zitat von Luiz Augusto von Dentz <lui...@gm...>: > > >> Btw, your new series looks good enough to be applied. > > > > > > Great. I've split it into three series and will send the next series > > > if the previous one is in git. > > > > > > You can selectivly (git reset ....) pull those patches from my > > > "releasing-2.0" branch at gitorious. > > > > Or at least this evening it will :) > > The current one is not fully up-to-date. > > Oh no, it was. Just the dumb corporate proxy :-( > I feel like talking to myself... oh wait... I do ;-) Or gitorious doesn't cope well... Whatever, when you pull it with git, it should have the right order :) HS |