| Message ID | 20220203184450.5491-3-scott.the.elm@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | rewrite avpriv_find_start_code() for clarity | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
Scott Theisen: > No functional change. > --- > libavcodec/utils.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index b19befef21..cb4437edc2 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -967,10 +967,14 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p, > } > } > > - p = FFMIN(p, end) - 4; > - *state = AV_RB32(p); > + if (p > end) > + p = end; > + // this will cause the last 4 bytes before end to be read, > + // i.e. no out of bounds memory access occurs > > - return p + 4; > + *state = AV_RB32(p - 4); // read the previous 4 bytes > + > + return p; > } > > AVCPBProperties *av_cpb_properties_alloc(size_t *size) Where exactly is the readability enhancement supposed to be? I only see the opposite: The earlier code spoke for itself; not this simplicity is obscured by lots of comments. Having to parse lots of comments makes the code harder to read. This is also my impression with your other clarification patches. - Andreas
diff --git a/libavcodec/utils.c b/libavcodec/utils.c index b19befef21..cb4437edc2 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -967,10 +967,14 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p, } } - p = FFMIN(p, end) - 4; - *state = AV_RB32(p); + if (p > end) + p = end; + // this will cause the last 4 bytes before end to be read, + // i.e. no out of bounds memory access occurs - return p + 4; + *state = AV_RB32(p - 4); // read the previous 4 bytes + + return p; } AVCPBProperties *av_cpb_properties_alloc(size_t *size)