Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] ARC: gcc - Fix SIZE_TYPE to be "unsigned int" instead of long unsigned int"
@ 2014-09-15 13:26 Alexey Brodkin
  2014-09-15 13:50 ` Thomas Petazzoni
  0 siblings, 1 reply; 6+ messages in thread
From: Alexey Brodkin @ 2014-09-15 13:26 UTC (permalink / raw)
  To: buildroot

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

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
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] ARC: gcc - Fix SIZE_TYPE to be "unsigned int" instead of long unsigned int"
  2014-09-15 13:26 [Buildroot] [PATCH] ARC: gcc - Fix SIZE_TYPE to be "unsigned int" instead of long unsigned int" Alexey Brodkin
@ 2014-09-15 13:50 ` Thomas Petazzoni
  2014-09-15 14:06   ` Alexey Brodkin
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2014-09-15 13:50 UTC (permalink / raw)
  To: buildroot

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] ARC: gcc - Fix SIZE_TYPE to be "unsigned int" instead of long unsigned int"
  2014-09-15 13:50 ` Thomas Petazzoni
@ 2014-09-15 14:06   ` Alexey Brodkin
  2014-09-15 14:29     ` Thomas Petazzoni
  0 siblings, 1 reply; 6+ messages in thread
From: Alexey Brodkin @ 2014-09-15 14:06 UTC (permalink / raw)
  To: buildroot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] ARC: gcc - Fix SIZE_TYPE to be "unsigned int" instead of long unsigned int"
  2014-09-15 14:06   ` Alexey Brodkin
@ 2014-09-15 14:29     ` Thomas Petazzoni
  2014-09-15 14:41       ` Alexey Brodkin
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2014-09-15 14:29 UTC (permalink / raw)
  To: buildroot

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] ARC: gcc - Fix SIZE_TYPE to be "unsigned int" instead of long unsigned int"
  2014-09-15 14:29     ` Thomas Petazzoni
@ 2014-09-15 14:41       ` Alexey Brodkin
  2014-09-15 14:55         ` Thomas Petazzoni
  0 siblings, 1 reply; 6+ messages in thread
From: Alexey Brodkin @ 2014-09-15 14:41 UTC (permalink / raw)
  To: buildroot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] ARC: gcc - Fix SIZE_TYPE to be "unsigned int" instead of long unsigned int"
  2014-09-15 14:41       ` Alexey Brodkin
@ 2014-09-15 14:55         ` Thomas Petazzoni
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2014-09-15 14:55 UTC (permalink / raw)
  To: buildroot

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-09-15 14:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-15 13:26 [Buildroot] [PATCH] ARC: gcc - Fix SIZE_TYPE to be "unsigned int" instead of long unsigned int" Alexey Brodkin
2014-09-15 13:50 ` Thomas Petazzoni
2014-09-15 14:06   ` Alexey Brodkin
2014-09-15 14:29     ` Thomas Petazzoni
2014-09-15 14:41       ` Alexey Brodkin
2014-09-15 14:55         ` Thomas Petazzoni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox