Small fixes #91
Small fixes #91
Conversation
bibendovsky
commented
May 4, 2023
bibendovsky
commented
- Fixed dereferencing null pointer in oggpack_writeinit
- Fixed type cast in ogg_page_serialno and ogg_page_pageno
| @@ -39,7 +39,7 @@ static const unsigned int mask8B[]= | |||
| void oggpack_writeinit(oggpack_buffer *b){ | |||
| memset(b,0,sizeof(*b)); | |||
| b->ptr=b->buffer=_ogg_malloc(BUFFER_INCREMENT); | |||
| b->buffer[0]='\0'; | |||
There was a problem hiding this comment.
This do not look correct. If malloc() fail, I doubt b->storage should get a non-zero value as the there is no storage available.
There was a problem hiding this comment.
This do not look correct. If malloc() fail, I doubt b->storage should get a non-zero value as the there is no storage available.
The fix do the check for a null. You can see this under the review conversation.
There was a problem hiding this comment.
I do not understand your comment. I see the addition of if(b->buffer) in your patch, but fail to see how this properly handle the case where malloc() return NULL, as it seem incomplete and returning a misleading state from the oggpack_writeinit() method. Why is your edition still modifying b->storage if there is no memory area pointed to by b->buffer?
There was a problem hiding this comment.
I created https://gitlab.xiph.org/xiph/ogg/-/merge_requests/26 with my proposed fix for the same issue, including updating the documentation to mention that the method can fail.
There was a problem hiding this comment.
I do not understand your comment. I see the addition of
if(b->buffer)in your patch, but fail to see how this properly handle the case where malloc() return NULL
Conditional operator if (b->buffer) prevents from writing '\0' into b->buffer when it is null.
but fail to see how this properly handle the case where malloc() return NULL, as it seem incomplete and returning a misleading state from the oggpack_writeinit() method.
The commit do exactly what it says - nothing more, nothing less.
I could add another commit to address this exact issue (the incomplete state).
Why is your edition still modifying b->storage if there is no memory area pointed to by b->buffer?
That was in original code.
|
[Boris I. Bendovsky]
The commit do exactly what it says - nothing more, nothing less.
No worries. I believe that part of this pull request is wrong, and
suggest to drop the commit from this set of changes. As mentioned
earlier, I passed what I believe is a more correct change upstream to
gitlab.xiph.org.
…--
Happy hacking
Petter Reinholdtsen
|
Inspired by proposal in xiph/ogg#91.
Inspired by proposal in xiph/ogg#91.
