Initialize pthread_t: CID 1375950

[araujo]

Summary

Initialize pthread according to CID 1375950.

[cem]

I don't think it's valid to pthread_join() a bogus pthread_t value, which isn't even necessarily a scalar type. I'd just use a new bool value, set it to true when the thread is created, and check before joining the tid value.

[araujo]

Check if pthread_create(3) returns 0.

[cem]

usr.sbin/bhyve/rfb.c

768 ↗

(On Diff #29474)

I'd drop this change — zero isn't necessarily a value that can be assigned to pthread_t.

919–920 ↗

(On Diff #29474)

This isn't quite right. In the error case pointed out in the email, error will be zero due to its initialization at the top of the function, but there will be no thread to join.

[araujo]

usr.sbin/bhyve/rfb.c

768 ↗

(On Diff #29474)

libzfs use it in the same way:

cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c:	pthread_t tid = 0;

919–920 ↗

(On Diff #29474)

pthread_create must return 0 if created successfully and the only goto done place is at the end of the switch case, error will be set during that time.

[cem]

usr.sbin/bhyve/rfb.c

768 ↗

(On Diff #29474)

Then libzfs is also wrong.

864 ↗

(On Diff #29474)

I think you are forgetting about this goto done;.

919–920 ↗

(On Diff #29474)

See the other goto done pointed out above.

[araujo]

usr.sbin/bhyve/rfb.c

919–920 ↗

(On Diff #29474)

Yes, you are right! I overlooked that.

[araujo]

Rewrite the attempt to fix CID 1375950.
I made couple tests using 'procstat -Ht <pid>' and also clang
static analisys didn't get anything else related with
the pthread_create(3).

[cem]

This addresses the Coverity warning, thanks! I'm not sure pthread_create failure is actually handled correctly, though.

usr.sbin/bhyve/rfb.c

883–886 ↗

(On Diff #29639)

Does it make sense to proceed here if pthread_create failed? Maybe instead:

if (perror != 0)
    goto done;
pthread_set_name_np(...);

[araujo]

usr.sbin/bhyve/rfb.c

883–886 ↗

(On Diff #29639)

Yes, I saw that too, but I'm not sure if pthread_create fail the best is go to 'done', I need to make some tests and maybe handle the pthread_create code error in a proper way.

@cem thank you again to point out all those issues and to make the review.