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

Increase size of buffer length variable #15

Merged
merged 1 commit into from May 24, 2016

Conversation

anoo1
Copy link
Contributor

@anoo1 anoo1 commented May 12, 2016

The latest Barreleye vpd contains additional custom fields in one
of the board sections, pushing the size of the section to over 0x100.

The current variable that stores the buffer size is set to be
uint8_t which doesn't fit the extended size. Changing it to uint32_t.

Signed-off-by: Adriana Kobylak anoo@us.ibm.com


This change is Reviewable

@@ -26,7 +26,7 @@ class ipmi_fru
std::string iv_name;

// Length of a specific fru area.
size_t iv_len;
uint32_t iv_len;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to change these: size_t is not 8 bits; it's definitely big enough.

@daxtens
Copy link

daxtens commented May 12, 2016

Review status: 0 of 3 files reviewed at latest revision, 4 unresolved discussions.


fru-area.H, line 29 [r1] (raw file):


        // Length of a specific fru area.
        uint32_t  iv_len;

I don't think you need to change these: size_t is absolutely big enough already.


frup.c, line 951 [r1] (raw file):

}

int parse_fru_area (const uint8_t area, const void* msgbuf, const uint32_t len, sd_bus_message* vpdtbl)

Is there any reason not use use size_t here?


Comments from Reviewable

@daxtens
Copy link

daxtens commented May 12, 2016

I was struggling to get reviewable to work so I commented on GitHub. Now I seem to have got Reviewable to work. Sorry for the duplication.

@anoo1
Copy link
Contributor Author

anoo1 commented May 13, 2016

The size of size_t is different in 32 and 64-bit architectures. There has been issues in the past in op-build where we had to change size_t to uint32_t to support compiling in 64-bit. So thought I'd make the change consistent and change the length to uint32_t.

Previously, daxtens (Daniel Axtens) wrote…

I was struggling to get reviewable to work so I commented on GitHub. Now I seem to have got Reviewable to work. Sorry for the duplication.


Review status: 0 of 3 files reviewed at latest revision, 4 unresolved discussions.


Comments from Reviewable

@daxtens
Copy link

daxtens commented May 13, 2016

That's a bit weird, but if we're using uint32_t elsewhere for sizes, I'm OK with doing it here.

Previously, anoo1 wrote…

The size of size_t is different in 32 and 64-bit architectures. There has been issues in the past in op-build where we had to change size_t to uint32_t to support compiling in 64-bit. So thought I'd make the change consistent and change the length to uint32_t.


Review status: 0 of 3 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

The latest Barreleye vpd contains additional custom fields in one
of the board sections, pushing the size of the buffer over 0x100.

The current variable that stores the buffer size is set to be
size_t, but the function using it crops it to uint8_t  which
doesn't fit the extended size. Changing the function to use size_t.

Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
@vishwabmc
Copy link

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.


frup.c, line 951 [r2] (raw file):

}

int parse_fru_area (const uint8_t area, const void* msgbuf, const size_t len, sd_bus_message* vpdtbl)

Looks good.


Comments from Reviewable

@williamspatrick williamspatrick merged commit 3a6177e into openbmc:master May 24, 2016
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

Successfully merging this pull request may close these issues.

None yet

4 participants