Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed to use CBOR encoding when compilig for GOARCH=386 #232

Closed
Niondir opened this issue Feb 19, 2018 · 8 comments

Comments

@Niondir
Copy link

commented Feb 19, 2018

I have no details error report yet but already like to mention that I'm not able to use the CBOR part when compiling for GOARCH=386

I'm using:

[[projects]]
  name = "github.com/ugorji/go"
  packages = ["codec"]
  revision = "9831f2c3ac1068a78f50999a30db84270f647af6"
  version = "v1.1"

Last working commit is: a5098f42c62243f0690aa971c0d2b42677bdacd9

some code to reproduce this (test works with 64 bit but breaks on 386):

type Config struct {
	Values []ConfigEntry
}

// Encoded as map
type MapBySliceType []interface{}

func (_ MapBySliceType) MapBySlice() {

}

func encodeCfg(cfg *Config) ([]byte, error) {
	var err error
	out := make([]byte, 0)
	handle := &codec.CborHandle{}
	enc := codec.NewEncoderBytes(&out, handle)

	s := MapBySliceType{}

	for _, v := range cfg.Values {
		s = append(s, v.Key)

		var converted interface{} = v.Value
		switch v.ValueType {
		// Working JSON types:
		case "[]uint8":
			// Byte arrays come as base64 strings
			switch t := v.Value.(type) {
			case string: // Expected
				converted, err = base64.StdEncoding.DecodeString(t)
				if err != nil {
					return nil, err
				}
			case []uint8: // Also supported
				converted = t
			default:
				return nil, fmt.Errorf("expected value of type %s encoded as Base64 string but got %s: %v", v.ValueType, reflect.TypeOf(v.Value), v.Value)
			}

		case "uint64", "int64", "uint32", "int32", "uint16", "int16", "uint8", "int8", "int":
			// Integers come as float64 (json number) or string encoded
			switch t := v.Value.(type) {
			case int:
			case uint64:
				converted = int(t)
			case float64:
				converted = int(t)
			case string:
				converted, err = strconv.Atoi(t)
				if err != nil {
					return nil, err
				}
			default:
				return nil, fmt.Errorf("expected value of type %s encoded as string or float64 (json 'number') but got %s: %v", v.ValueType, reflect.TypeOf(v.Value), v.Value)
			}
		}

		s = append(s, converted)
	}

	enc.Encode(s)

	return out, nil
}

func TestEncodeConfig(t *testing.T) {
	cfg := &Config{}
	cfg.Values = append(cfg.Values, ConfigEntry{Key: "key1", Value: "val1"})
	cfg.Values = append(cfg.Values, ConfigEntry{Key: "key2", Value: "val2"})

	bin, err := encodeCfg(cfg)
	if err != nil {
		t.Error(err) // EOF
	}
}
@ugorji

This comment has been minimized.

Copy link
Owner

commented Feb 19, 2018

Please specify the reproducer: execution file(s), executable line, output expected, output got.

E.g using a main.go containing output below, I ran "go run main.go", or using a file_test.go, I ran "go test" and received output below, but expected output below.

This makes it super easy to reproduce your issue, meaning I can fix it faster.

@ugorji

This comment has been minimized.

Copy link
Owner

commented Feb 20, 2018

Also, this reproducer is complex. I don't even know what it's supposed to be doing, so I can't say if the error is in your code or the lib or if any error exists.

@ugorji ugorji added the NeedMoreInfo label Feb 20, 2018

ugorji added a commit that referenced this issue Feb 21, 2018

@ugorji

This comment has been minimized.

Copy link
Owner

commented Feb 21, 2018

Also updated by 2af342e

@ugorji

This comment has been minimized.

Copy link
Owner

commented Feb 21, 2018

Closing this for now. I am not sure if this fixes your issues, but these are the issues I found by doing a number of 386 (32-bit) tests.

Please, Add a comment if you still see your issue, and attach a proper reproducer.

Thanks.

@ugorji ugorji closed this Feb 21, 2018

@Niondir

This comment has been minimized.

Copy link
Author

commented Feb 21, 2018

The testcase I added runs on 64bit compilation but breaks with EOF error on 386, so you can just run the test to reproduce the issue.

If you have any other problems running this test, let me know but closing this issue without even trying it seems a bit too fast.

@ugorji

This comment has been minimized.

Copy link
Owner

commented Feb 23, 2018

I don't understand the test case. It's obscure. I need you to create a test case that just focuses on the problem, else I might find myself spending cycles trying to understand whether this was a user error or not, and that is me doing your debugging for you and not me fixing a clearly identified issue.

That is why this is not a reproducer. Your test case addresses a ConfigEntry type which is not even in the code snippet.

Please - if you want a fix, you have to spend some time to make it easy to reproduce. Keeping it closed until you provide a true reproducer as described in #issuecomment-366818313

@Niondir

This comment has been minimized.

Copy link
Author

commented Mar 7, 2018

@ugorji it's fixed on master. But the bug appears in 1.0 tag. Maybe it makes sense to create another release tag for the latest version.

@ugorji

This comment has been minimized.

Copy link
Owner

commented Mar 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.