Message ID | 1410787585-20387-1-git-send-email-abrodkin@synopsys.com |
---|---|
State | Superseded |
Headers | show |
Dear Alexey Brodkin, On Mon, 15 Sep 2014 17:26:25 +0400, Alexey Brodkin wrote: > This makes size_t to be "unsigned" ssize_t which makes happy compiler on data > type checks. > > Fix is taken from current development branch of GCC for ARC and will be a > part of the next release of ARC tools, so at that point patch should be dropped. > > https://github.com/foss-for-synopsys-dwc-arc-processors/gcc/commit/249f040299402647525c3f15b79d319fa7acddd3 > > Fixes http://autobuild.buildroot.net/results/405/405da9a945511329929b18740b983c51b8dcc43e > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > > Cc: Anton Kolesov <akolesov@synopsys.com> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Cc: Peter Korsgaard <peter@korsgaard.com> > --- > package/gcc/arc-2014.08/200-size_type_unsigned_int.patch | 11 +++++++++++ > 1 file changed, 11 insertions(+) > create mode 100644 package/gcc/arc-2014.08/200-size_type_unsigned_int.patch Thanks, looks good, but... > > diff --git a/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch b/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch > new file mode 100644 > index 0000000..78d4b10 > --- /dev/null > +++ b/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch Patch lacks a description + SoB. Thanks! Thomas
Hi Thomas, On Mon, 2014-09-15 at 15:50 +0200, Thomas Petazzoni wrote: > Dear Alexey Brodkin, > > diff --git a/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch b/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch > > new file mode 100644 > > index 0000000..78d4b10 > > --- /dev/null > > +++ b/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch > > Patch lacks a description + SoB. Well first of all IMHO patch name is self-explanatory. Than I didn't want to duplicate the same text in commit message and in patch itself. If you think this is helpful I will happily add the same text and URL in the patch. Let me know what do you think about it. -Alexey
Dear Alexey Brodkin, On Mon, 15 Sep 2014 14:06:43 +0000, Alexey Brodkin wrote: > > > diff --git a/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch b/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch > > > new file mode 100644 > > > index 0000000..78d4b10 > > > --- /dev/null > > > +++ b/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch > > > > Patch lacks a description + SoB. > > Well first of all IMHO patch name is self-explanatory. > Than I didn't want to duplicate the same text in commit message and in > patch itself. > > If you think this is helpful I will happily add the same text and URL in > the patch. > > Let me know what do you think about it. We have a stupid rule, and stupid rules are meant to be complied with :-) Joke aside, I think what you mentioned in the commit log is useful: what the patch does, and the fact that it's already "upstream" and therefore should be dropped at the next version bump of the ARC gcc compiler. Thanks! Thomas
Hi Thomas, On Mon, 2014-09-15 at 16:29 +0200, Thomas Petazzoni wrote: > Dear Alexey Brodkin, > > On Mon, 15 Sep 2014 14:06:43 +0000, Alexey Brodkin wrote: > > > > > diff --git a/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch b/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch > > > > new file mode 100644 > > > > index 0000000..78d4b10 > > > > --- /dev/null > > > > +++ b/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch > > > > > > Patch lacks a description + SoB. > > > > Well first of all IMHO patch name is self-explanatory. > > Than I didn't want to duplicate the same text in commit message and in > > patch itself. > > > > If you think this is helpful I will happily add the same text and URL in > > the patch. > > > > Let me know what do you think about it. > > We have a stupid rule, and stupid rules are meant to be complied > with :-) Understood - uniformity is good even if the rule itself is not that good :) > Joke aside, I think what you mentioned in the commit log is useful: > what the patch does, and the fact that it's already "upstream" and > therefore should be dropped at the next version bump of the ARC gcc > compiler. Funny enough that initially I had the same comment in the patch itself, but while looking at output of "git format-patch" I found myself a bit frustrated - if I were a reviewer I would say that submitted "full" patch looks ridiculous. Then I removed comment from commit message. That made me think that you guys won't apply a patch that won't show any useful info in git log. So I decided to move a message to the Buildroot commit. This made sense for me because at least when I bump version of a package I look at history of this package and this way try to figure out what's still required and what's not. Moreover looking at the next folder "package/gcc/4.9.1" I found that half of patches don't have any comment inside, some have URLs to bugs/commits in GCC, some are full-scale patches with verbose description. Now you may understand my frustration. Anyways, I'm about to send v2 with comments in both commit message and patch itself. Thanks for your time. -Alexey
Dear Alexey Brodkin, On Mon, 15 Sep 2014 14:41:57 +0000, Alexey Brodkin wrote: > > We have a stupid rule, and stupid rules are meant to be complied > > with :-) > > Understood - uniformity is good even if the rule itself is not that > good :) Absolutely :) > Funny enough that initially I had the same comment in the patch itself, > but while looking at output of "git format-patch" I found myself a bit > frustrated - if I were a reviewer I would say that submitted "full" > patch looks ridiculous. > > Then I removed comment from commit message. That made me think that you > guys won't apply a patch that won't show any useful info in git log. > > So I decided to move a message to the Buildroot commit. This made sense > for me because at least when I bump version of a package I look at > history of this package and this way try to figure out what's still > required and what's not. Hehe, funny. No problem if there is actually some duplication between the git commit log, and the patch description itself. > Moreover looking at the next folder "package/gcc/4.9.1" I found that > half of patches don't have any comment inside, some have URLs to > bugs/commits in GCC, some are full-scale patches with verbose > description. Half of the patches are carried over since gcc 4.2 or so, in a pre-2008 era, at a point where the state of Buildroot was clearly not as good as it is today. I think you wouldn't want to look at a pre-2008 Buildroot tree :-) That being said, is your comment an indication that you volunteer to do some research about these patches, write proper descriptions, or even better help getting them merged upstream? :-) Joke aside again, it's by having less and less patches with no description that at some point all patches in Buildroot will have a description. This way nobody will feel your frustration because it will just appear to be the "normal thing". > Now you may understand my frustration. Yes, of course, I do understand. Thomas
diff --git a/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch b/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch new file mode 100644 index 0000000..78d4b10 --- /dev/null +++ b/package/gcc/arc-2014.08/200-size_type_unsigned_int.patch @@ -0,0 +1,11 @@ +--- a/gcc/config/arc/arc.h ++++ b/gcc/config/arc/arc.h +@@ -487,7 +487,7 @@ if (GET_MODE_CLASS (MODE) == MODE_INT \ + /* Define this as 1 if `char' should by default be signed; else as 0. */ + #define DEFAULT_SIGNED_CHAR 0 + +-#define SIZE_TYPE "long unsigned int" ++#define SIZE_TYPE "unsigned int" + #define PTRDIFF_TYPE "long int" + #define WCHAR_TYPE "int" + #define WCHAR_TYPE_SIZE 32
This makes size_t to be "unsigned" ssize_t which makes happy compiler on data type checks. Fix is taken from current development branch of GCC for ARC and will be a part of the next release of ARC tools, so at that point patch should be dropped. https://github.com/foss-for-synopsys-dwc-arc-processors/gcc/commit/249f040299402647525c3f15b79d319fa7acddd3 Fixes http://autobuild.buildroot.net/results/405/405da9a945511329929b18740b983c51b8dcc43e Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Cc: Anton Kolesov <akolesov@synopsys.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: Peter Korsgaard <peter@korsgaard.com> --- package/gcc/arc-2014.08/200-size_type_unsigned_int.patch | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 package/gcc/arc-2014.08/200-size_type_unsigned_int.patch