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

PVS studio report #1754

Open
shlyakpavel opened this Issue Dec 25, 2018 · 1 comment

Comments

4 participants
@shlyakpavel
Copy link
Contributor

shlyakpavel commented Dec 25, 2018

Description of the problem

I have checked some parts of a project with a static code analyzer. I will leave PRs for some of the issues
This can cause null pointer dereference. The idea of using goto here is wrong

if (pv->stream->in_queue == NULL) goto fail;

Null pointer is freed here
free( ts->task_threads_args );

Pointer is first dereferenced, then checked against null

HandBrake/libhb/decvobsub.c

Lines 169 to 171 in a482041

*buf_out = Decode( w );
if( buf_out && *buf_out )

The condition is checked twice

HandBrake/libhb/fifo.c

Lines 920 to 924 in a482041

if( f->size < 1 )
{
f->wait_empty = 1;
hb_cond_timedwait( f->cond_empty, f->lock, FIFO_TIMEOUT );
if( f->size < 1 )

The same code here

HandBrake/libhb/fifo.c

Lines 974 to 978 in a482041

if( f->size < 1 )
{
f->wait_empty = 1;
hb_cond_timedwait( f->cond_empty, f->lock, FIFO_TIMEOUT );
if( f->size < 1 )

I do not understand this, actually. Looks like nonsense

HandBrake/libhb/param.c

Lines 803 to 810 in a482041

else if (!strcasecmp(preset, "stronger"))
{
strength[0] = strength[1] = 0.375;
}
else if (!strcasecmp(preset, "stronger"))
{
strength[0] = strength[1] = 0.375;
}

Same here

HandBrake/gtk/src/callbacks.c

Lines 4515 to 4522 in a482041

if (!active)
{
gtk_tool_button_set_label(button, "Activity");
}
else
{
gtk_tool_button_set_label(button, "Activity");
}

And here

HandBrake/gtk/src/callbacks.c

Lines 4658 to 4665 in a482041

if (!active)
{
show_hide = _("Queue");
}
else
{
show_hide = _("Queue");
}

And here

HandBrake/gtk/src/preview.c

Lines 1108 to 1115 in a482041

if (!active)
{
gtk_tool_button_set_label(button, "Preview");
}
else
{
gtk_tool_button_set_label(button, "Preview");
}

Thanks for providing a great product!
Have a nice day)

bradleysepos added a commit that referenced this issue Dec 26, 2018

@bradleysepos

This comment has been minimized.

Copy link
Member

bradleysepos commented Dec 26, 2018

Thanks for the report. So far, I've fixed the nonsense in param.c in 9508b54.

@bradleysepos bradleysepos added this to the 1.2.1 milestone Dec 26, 2018

bradleysepos added a commit to bradleysepos/HandBrake that referenced this issue Dec 26, 2018

@sr55 sr55 added the Bug label Dec 30, 2018

shlyakpavel added a commit to shlyakpavel/HandBrake that referenced this issue Jan 5, 2019

Cleanup preview.c
Refer HandBrake#1754 for more details

jstebbins added a commit that referenced this issue Jan 5, 2019

Cleanup preview.c
Refer #1754 for more details

shlyakpavel added a commit to shlyakpavel/HandBrake that referenced this issue Jan 5, 2019

Cleanup callbacks.c
Refer HandBrake#1754 for more details

jstebbins added a commit that referenced this issue Jan 5, 2019

Cleanup callbacks.c
Refer #1754 for more details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment