FAQ
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com,
vova616@gmail.com),

Please take another look.


http://codereview.appspot.com/6782097/

Search Discussions

  • Vova616 at Nov 21, 2012 at 1:34 pm
    Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com,
    vova616@gmail.com),

    Please take another look.


    http://codereview.appspot.com/6782097/
  • Vova616 at Nov 21, 2012 at 1:34 pm
    Reviewers: golang-dev_googlegroups.com,

    Message:
    Hello golang-dev@googlegroups.com (cc: vova616@gmail.com),

    I'd like you to review this change to
    https://code.google.com/p/go


    Description:
    cmd/cgo: bool alignment/padding issue.
    Fixes issue 4417.

    Please review this at http://codereview.appspot.com/6782097/

    Affected files:
    M src/cmd/cgo/gcc.go
    A test/fixedbugs/issue4417.go


    Index: src/cmd/cgo/gcc.go
    ===================================================================
    --- a/src/cmd/cgo/gcc.go
    +++ b/src/cmd/cgo/gcc.go
    @@ -771,8 +771,8 @@
    "-o" + gccTmp(), // write object to tmp
    "-gdwarf-2", // generate DWARF v2 debugging
    symbols
    "-fno-eliminate-unused-debug-types", // gets rid of e.g. untyped enum
    otherwise
    - "-c", // do not link
    - "-xc", // input language is C
    + "-c", // do not link
    + "-xc", // input language is C
    }
    c = append(c, p.GccOptions...)
    c = append(c, p.gccMachine()...)
    @@ -1078,7 +1078,7 @@

    case *dwarf.BoolType:
    t.Go = c.bool
    - t.Align = c.ptrSize
    + t.Align = 1

    case *dwarf.CharType:
    if t.Size != 1 {
    Index: test/fixedbugs/issue4417.go
    ===================================================================
    new file mode 100644
    --- /dev/null
    +++ b/test/fixedbugs/issue4417.go
    @@ -0,0 +1,48 @@
    +// run
    +
    +// Copyright 2012 The Go Authors. All rights reserved.
    +// Use of this source code is governed by a BSD-style
    +// license that can be found in the LICENSE file.
    +
    +// Issue 4417: cmd/cgo: bool alignment/padding issue.
    +// bool alignment is wrong and causing wrong arguments when calling
    functions.
    +//
    +
    +package main
    +
    +/*
    +#include <stdbool.h>
    +
    +static int c_bool(bool a, bool b, int c, bool d, bool e) {
    + return c;
    +}
    +*/
    +import "C"
    +
    +func main() {
    + b := C.c_bool(true, true, 10, true, false)
    + if b != 10 {
    + print("found ", b, " excepted 10\n")
    + panic("failure.")
    + }
    + b = C.c_bool(true, true, 5, true, true)
    + if b != 5 {
    + print("found ", b, " excepted 5\n")
    + panic("failure.")
    + }
    + b = C.c_bool(true, true, 3, true, false)
    + if b != 3 {
    + print("found ", b, " excepted 3\n")
    + panic("failure.")
    + }
    + b = C.c_bool(false, false, 1, true, false)
    + if b != 1 {
    + print("found ", b, " excepted 1\n")
    + panic("failure.")
    + }
    + b = C.c_bool(false, true, 200, true, false)
    + if b != 200 {
    + print("found ", b, " excepted 200\n")
    + panic("failure.")
    + }
    +}
    \ No newline at end of file
  • Iant at Nov 21, 2012 at 2:06 pm
    Have you signed the CLA?


    https://codereview.appspot.com/6782097/diff/2001/test/fixedbugs/issue4417.go
    File test/fixedbugs/issue4417.go (right):

    https://codereview.appspot.com/6782097/diff/2001/test/fixedbugs/issue4417.go#newcode25
    test/fixedbugs/issue4417.go:25: print("found ", b, " excepted 10\n")
    s/excepted/expected/ here and below

    https://codereview.appspot.com/6782097/
  • Minux at Nov 21, 2012 at 3:36 pm
    please put the cgo test file to misc/cgo/test/.
  • Bradfitz at Nov 21, 2012 at 4:16 pm
  • Iant at Nov 21, 2012 at 7:49 pm
    Please change the first line of the CL description to

    cmd/cgo: fix alignment of bool

    Thanks.

    For the record, I checked GCC, and the alignment of bool should be 1 on
    every system except PowerPC Darwin, which we not care about.

    https://codereview.appspot.com/6782097/
  • Iant at Nov 21, 2012 at 8:53 pm
  • Iant at Nov 21, 2012 at 8:54 pm
  • Iant at Nov 21, 2012 at 9:09 pm
    *** Submitted as
    http://code.google.com/p/go/source/detail?r=dfb4a24735c0 ***

    cmd/cgo: fix alignment of bool.
    Fixes issue 4417.

    R=golang-dev, iant, minux.ma, bradfitz
    CC=golang-dev, vova616
    http://codereview.appspot.com/6782097

    Committer: Ian Lance Taylor <iant@golang.org>


    http://codereview.appspot.com/6782097/
  • Vova616 at Nov 21, 2012 at 11:30 pm

    On 2012/11/21 14:06:11, iant wrote:
    Have you signed the CLA?

    https://codereview.appspot.com/6782097/diff/2001/test/fixedbugs/issue4417.go
    File test/fixedbugs/issue4417.go (right):

    https://codereview.appspot.com/6782097/diff/2001/test/fixedbugs/issue4417.go#newcode25
    test/fixedbugs/issue4417.go:25: print("found ", b, " excepted 10\n")
    s/excepted/expected/ here and below
    how do I do sign the CLA?

    https://codereview.appspot.com/6782097/
  • Minux at Nov 21, 2012 at 4:45 pm

    On Thu, Nov 22, 2012 at 12:43 AM, wrote:

    how do I do sign the CLA?
    http://golang.org/doc/contribute.html#copyright
  • Vova616 at Nov 21, 2012 at 11:30 pm

    On 2012/11/21 19:49:32, iant wrote:
    Please change the first line of the CL description to
    cmd/cgo: fix alignment of bool
    Thanks.
    For the record, I checked GCC, and the alignment of bool should be 1 on every
    system except PowerPC Darwin, which we not care about.
    Great, I also checked it a little and it seems to be right.
    I have changed the description, thanks.

    https://codereview.appspot.com/6782097/
  • Vova616 at Nov 21, 2012 at 11:30 pm
    Hello golang-dev@googlegroups.com, iant@golang.org, minux.ma@gmail.com,
    bradfitz@golang.org (cc: golang-dev@googlegroups.com,
    vova616@gmail.com),

    Please take another look.


    http://codereview.appspot.com/6782097/
  • Vova616 at Nov 21, 2012 at 11:30 pm
    Hello golang-dev@googlegroups.com, iant@golang.org, minux.ma@gmail.com,
    bradfitz@golang.org (cc: golang-dev@googlegroups.com,
    vova616@gmail.com),

    Please take another look.


    http://codereview.appspot.com/6782097/
  • Vova616 at Nov 21, 2012 at 11:30 pm

    On 2012/11/21 16:45:02, minux wrote:
    On Thu, Nov 22, 2012 at 12:43 AM, wrote:

    how do I do sign the CLA?
    http://golang.org/doc/contribute.html#copyright
    Thanks, my CLA submission is being processed.

    https://codereview.appspot.com/6782097/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedNov 21, '12 at 1:34p
activeNov 21, '12 at 11:30p
posts16
users4
websitegolang.org

4 users in discussion

Vova616: 8 posts Iant: 5 posts Minux: 2 posts Bradfitz: 1 post

People

Translate

site design / logo © 2022 Grokbase