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

Option to fill in zero bytes for fixed size data types when undefined? #81

Open
LiamKarlMitchell opened this issue Jun 23, 2017 · 5 comments

Comments

@LiamKarlMitchell
Copy link

LiamKarlMitchell commented Jun 23, 2017

Oh, sorry about custom type it is just an example, I think this should be an option for any type that can have a count set to a fixed number.

If you have a definition with a fixed size.

{
  "name": "foo",
  "type": [
    "fcstring",
    {
      "count": 4,
      "trim": true
    }
  ]
}

And wanted to serialize some object where it is undefined
{}

Then the buffer should zero fill.

<Buffer 00 00 00 00>

Example with buffer data type.

{
  "name": "foo",
  "type": [
    "buffer",
    {
      "count": 4
    }
  ]
}

<Buffer 00 00 00 00>

And applicable to arrays as well.

Would such an option be possible?
Maybe when setting up the serializer.

@hansihe
Copy link
Member

hansihe commented Jun 24, 2017

I disagree with this. If there is data missing that needs to be written, the serializer should throw a hard error.

Making guesses and zero-filling/doing anything else seems to me like it would make it really easy to introduce hard to debug logic errors. It is better to just throw an error, and the user can immediately see that they made a mistake and correct it.

@Saiv46
Copy link
Contributor

Saiv46 commented Jun 13, 2020

Maybe you should add "padding" property to datatype, but it would introduce various bugs (in case of "pstring" - it would deserialize to "\x00\x00\x00\x00", not empty string).

@LiamKarlMitchell
Copy link
Author

LiamKarlMitchell commented Jun 14, 2020

Yeah I trim the null bytes so its not an issue.
Sending buffers to a C++ made application with fixed sized structures so it wants this 0 padding.

@Saiv46
Copy link
Contributor

Saiv46 commented Jun 14, 2020

By default interpreter/compiler just ignores count and in future, it'll throw an error.

// TODO: Throw

https://github.com/ProtoDef-io/node-protodef/pull/107/files#diff-2b4ca49d4bb0a774c4d4c1672d7aa781R86

@rom1504 @Karang Should we change this behavior?

@roblabla
Copy link
Collaborator

roblabla commented Jun 15, 2020

@LiamKarlMitchell For structure padding specifically, it might be worth thinking about some kind of data-type that's basically a "zeroed byte". The idea is that you'd never specify it yourself, it would always be automatically set to 0.

The concept could be more generally extended to setting default values for the existing primitives. E.G. those two would be equivalent:

struct mystruct {
    u16 a;
    // two bytes of padding due to C struct layout
    u32 b;
};
{
  "padding_byte": [
    "u8",
    {
      "value": 0
    }
  ],
  "mystruct": ["container", [
    { "name": "a", "type": "u16" },
    { "type": "padding_byte" },
    { "type": "padding_byte" },
    { "name": "b", "type": "u32" },
  ]]
}

@Saiv46 TBH, setting a count manually should never be allowed to begin with. When I designed count, I'm pretty sure I expected it to be derived automatically from the backing array all the time, and never be manually set. I'm a bit ashamed I left it as a TODO and never implemented the throw :').

I don't think automatic zero-filling makes too much sense the way it's presented right now. I'd like to see some more concrete use-cases that couldn't be better served by an alternative proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants