Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#8156 closed defect (invalid)

PVS-studio big list of errors

Reported by: Balling Owned by:
Priority: normal Component: undetermined
Version: unspecified Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description (last modified by Balling)

Use keywords with Ctrl-F: "function" and "twice" and "false", "true"

For example:
./libavfilter/af_headphone.c 713 warn V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 712, 713.
./libavfilter/vf_avgblur.c 173 warn V519 The 'ptr' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 173, 173.
./libavfilter/vf_avgblur.c 174 warn V519 The 'ptr' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 174, 174.

There is another 27 such twice values. They are all mistakes...

./fftools/ffplay.c 2496 warn V763 Parameter 'wanted_nb_channels' is always rewritten in function body before being used. There is also such things.

./libavcodec/dirac_dwt_template.c 410 warn V751 Parameter 'height' is not used inside function body. And like this.
./libavcodec/j2kenc.c 524 err V610 Undefined behavior. Check the shift operator '<<'. The left operand is negative ('6 * i - (9 << (7 - 1) - 1)' = [-288..474]). Such things...
./libavcodec/j2kenc.c 323 err V523 The 'then' statement is equivalent to the 'else' statement. And such...
./libavcodec/encode.c 169 warn V581 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 162, 169.
./libavcodec/dvenc.c 410 err V767 Suspicious access to element of 'size' array by a constant index inside a loop.

./libavcodec/dnxhdenc.c 233 warn V728 An excessive check can be simplified. The or operator is surrounded by opposite expressions '!offset' and 'offset'.
./libavcodec/dnxhdenc.c 234 warn V728 An excessive check can be simplified. The or operator is surrounded by opposite expressions '!run' and 'run'.
./libavfilter/vf_overlay.c 491 warn V728 An excessive check can be simplified. The or operator is surrounded by opposite expressions '!vsub' and 'vsub'.
./libavfilter/f_loop.c 381 warn V728 An excessive check can be simplified. The or operator is surrounded by opposite expressions 's->nb_frames < s->size' and 's->nb_frames >= s->size'. These are just great.

Attachments (1)

project2019.tasks (450.8 KB ) - added by Balling 5 years ago.

Download all attachments as: .zip

Change History (14)

by Balling, 5 years ago

Attachment: project2019.tasks added

comment:1 by Balling, 5 years ago

https://github.com/FFmpeg/FFmpeg/blob/6f4ec4d909cc6dae5ce51f95e7518f2553215dc8/libavcodec/tiff.c#L1429-L1432
Wow, this is funny... There are identical sub-expressions...

./libavformat/icoenc.c	48	err	V501 There are identical sub-expressions to the left and to the right of the != operator: AV_PIX_FMT_BGRA != AV_PIX_FMT_BGRA
./libavformat/hevc.c	543	err	V501 There are identical sub-expressions to the left and to the right of the && operator: get_bits1(gb) && get_bits1(gb)
./libavfilter/dnn/dnn_backend_native.h	39	err	V501 There are identical sub-expressions to the left and to the right of the | operator: DOT_INPUT | DOT_INPUT
./libswscale/swscale_unscaled.c	2029	err	V501 There are identical sub-expressions '(srcFormat == AV_PIX_FMT_RGBA64LE && dstFormat == AV_PIX_FMT_RGBA64BE)' to the left and to the right of the || operator.
./libswscale/swscale_unscaled.c	2029	err	V501 There are identical sub-expressions '(srcFormat == AV_PIX_FMT_BGRA64BE && dstFormat == AV_PIX_FMT_BGRA64LE)' to the left and to the right of the || operator.
./libswscale/swscale_unscaled.c	2029	err	V501 There are identical sub-expressions '(srcFormat == AV_PIX_FMT_BGRA64LE && dstFormat == AV_PIX_FMT_BGRA64BE)' to the left and to the right of the || operator.
./libswscale/swscale_unscaled.c	2029	err	V501 There are identical sub-expressions to the left and to the right of the || operator.
./libswscale/swscale_unscaled.c	2029	err	V501 There are identical sub-expressions '(srcFormat == AV_PIX_FMT_RGBA64BE && dstFormat == AV_PIX_FMT_RGBA64LE)' to the left and to the right of the || operator.

By the way first thanks to patch to https://patchwork.ffmpeg.org/patch/15105/ !!
And this one https://github.com/FFmpeg/FFmpeg/commit/7a7aa4f79e506eff8bddb971da2b7cf0dac7d009

Ah yes I should've said I compiled against 77937a42e7127271bd50d7f8035c3ebd5a1047c5

Last edited 5 years ago by Balling (previous) (diff)

comment:2 by Balling, 5 years ago

Status: newopen

So, in commits https://github.com/FFmpeg/FFmpeg/commit/7a7aa4f79e506eff8bddb971da2b7cf0dac7d009
and https://github.com/FFmpeg/FFmpeg/commit/dc3325076597b41e0e05f201df2a8a84f588006d
you already solved my comments on twice assignment in first comment. So I will add all other cases of "twice":

./libavformat/hlsenc.c	2408	note	V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 2406, 2408.
./libavfilter/vf_ciescope.c	1195	warn	V519 The 'wx' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1193, 1195.
./libavfilter/vf_ciescope.c	1196	warn	V519 The 'wy' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1194, 1196.
./libavfilter/vf_ciescope.c	1202	warn	V519 The 'wx' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1200, 1202.
./libavfilter/vf_ciescope.c	1203	warn	V519 The 'wy' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1201, 1203.
./libavformat/asfdec_o.c	1147	warn	V519 The 'asf_pkt->duration' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1144, 1147.
./libavformat/asfdec_o.c	1561	warn	V519 The 'pkt->duration' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1558, 1561.
./libavformat/dhav.c	198	warn	V519 The 'ret' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 189, 198.
./libavformat/segment.c	393	warn	V519 The 'seg->segment_list_entries_end' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 390, 393.
./libavcodec/dirac_parser.c	263	warn	V519 The '* poutbuf' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 250, 263.
./libavcodec/dirac_parser.c	264	warn	V519 The '* poutbuf_size' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 251, 264.
./libavcodec/hevcpred_template.c	224	warn	V519 The 'left[- 1]' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 209, 224.
./libavcodec/snowenc.c	344	warn	V519 The 'ref_score' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 343, 344.
./libavcodec/vaapi_encode_mpeg2.c	331	warn	V519 The 'ph->forward_f_code' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 329, 331.
./fftools/ffmpeg_filter.c	731	warn	V519 The 'ifilter->width' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 728, 731.
./fftools/ffmpeg_filter.c	732	warn	V519 The 'ifilter->height' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 729, 732.
Last edited 5 years ago by Balling (previous) (diff)

comment:4 by Hendrik, 5 years ago

There is a continue in the loop above the break, the code may not be entirely clear, but it functions as expected.

comment:5 by Balling, 5 years ago

https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/avf_showfreqs.c#L198
This should be if (!s->avg_data) Solved in a46ee096d1a57cf7e00550681c5b477eeedf9eef

https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/window_func.h#L136
Wow, what is the point? norm = norm? Fixed in fa045c3ce288e3ffb6292cb8c0a1b21481de6974.
https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/af_afir.c#L565

s->eof_coeffs is always true because were it false we would have already return on line 561.

https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/avf_showspectrum.c#L1064
This !!s->stop should be just 1))

./libavfilter/f_loop.c	187	warn	V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions 's->nb_samples < s->size' and 's->nb_samples >= s->size'. 
./libavfilter/f_loop.c	381	warn	V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions 's->nb_frames < s->size' and 's->nb_frames >= s->size'. 

These were in first comment...

https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/src_movie.c#L259
timestamp > 0 is always true because timestamp = movie->seek_point; which is > 0

Wow, so many false positives((

Last edited 4 years ago by Balling (previous) (diff)

comment:6 by Balling, 5 years ago

https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/vf_dedot.c#L343
!s->eof is always true.
https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/vf_deflicker.c#L393
I suppose no need to check metadata because we do dereference pointer to struct on previous line which is not NULL and a field inside struct will not have NULL address ever...
https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/vf_detelecine.c#L224
Not need to check nskip_fields is unsigned int... But maybe add a comment))
https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/vf_fftdnoiz.c#L624
We are inside if which checks for this! s->nb_next > 0 can be deleted

https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/vf_fftfilt.c#L324
This else is not possible)) I suppose. All variants are already checked.

https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/vf_fieldmatch.c#L779-L781
Strange that this is the same as on lines 765, 766

https://github.com/FFmpeg/FFmpeg/blob/ef50cf7b32b91af303e37236f22e2e89971a84b7/libavfilter/vf_hqdn3d.c#L183
Negative value left shift... You like it)) Just add brackets -(256<<LUT_BITS)

Last edited 5 years ago by Balling (previous) (diff)

comment:7 by Balling, 5 years ago

As said here https://patchwork.ffmpeg.org/patch/15133/ there is indeed and empty check present. So I will show such expressions:

./libavformat/anm.c	114	warn	V555 The expression '(anm->nb_records - 1) > (0)' will work as 'anm->nb_records != 1'.
./libavformat/mp3enc.c	359	warn	V555 The expression of the 'A - B > 0' kind will work as 'A != B'.
./libavformat/wtvdec.c	697	warn	V555 The expression '(size - consumed) > (0)' will work as 'size != consumed'.
./libavformat/wtvdec.c	701	warn	V555 The expression '(size - consumed) > (0)' will work as 'size != consumed'.
./libavcodec/4xm.c	453	warn	V555 The expression of the 'A - B > 0' kind will work as 'A != B'.
./libavcodec/bmp.c	186	warn	V555 The expression 'hsize - ihsize - 14 > 0' will work as 'hsize - ihsize != 14'.
./libavcodec/bmp.c	193	warn	V555 The expression 'hsize - ihsize - 14 > 0' will work as 'hsize - ihsize != 14'.
./libavcodec/cbs_av1.c	231	warn	V555 The expression 'w - 1 > 0' will work as 'w != 1'.
./libavcodec/opus_rc.c	394	warn	V555 The expression '(size - rc->rb.bytes) > (0)' will work as 'size != rc->rb.bytes'.
./libavcodec/opus_rc.c	398	warn	V555 The expression '(rc->rb.bytes - lap) > (0)' will work as 'rc->rb.bytes != lap'.

Go forth!

comment:8 by Balling, 5 years ago

https://github.com/FFmpeg/FFmpeg/blob/ffb32d35eee616f79a37c4c96f37f2697932cc32/libavformat/nsvdec.c#L472
Strange nsv->vheight != vwidth

./libavformat/rtpdec_jpeg.c 316 warn V507 Pointer to local array 'new_qtables' is stored outside the scope of this array. Such a pointer will become invalid.
./libavformat/rtpdec_mpa_robust.c 76 warn V763 Parameter 'len' is always rewritten in function body before being used.
./libavformat/rtpenc.c 585 warn V1037 Two or more case-branches perform the same actions. Check lines: 585, 604

comment:10 by Carl Eugen Hoyos, 5 years ago

Resolution: invalid
Status: openclosed

I don't think this is a valid ticket.

in reply to:  10 comment:11 by Balling, 5 years ago

Replying to cehoyos:

I don't think this is a valid ticket.

Why?

comment:12 by Carl Eugen Hoyos, 5 years ago

One ticket per issue please.
But if you know how to fix an issue, please send a patch made with git format-patch to the development mailing list instead.

comment:13 by Balling, 4 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.