Hi,
we would like to suggest a few fixes for minor bugs and compiler warnings in libisoburn. Those were found by static analysis tools like Coverity, CLang and gcc warnings.
1) Bitwise operations - shift 1 << 31 is used on multiple places through xorriso, which is an undefined operation; fix: make 1 unsigned: 1u << 31
2) Absolute value - you are using abs() to calculate the absolute value of r1count and r2count, which are of type long int, so it could be truncated; fix: use labs()
3) Dereference of NULL pointers - in xorriso/opts_d_h.c, first_job pointer can be in case of failure of Findjob_new a NULL pointer, which is dereferenced in ex:
- in xorriso/opts_i_o.c, also descr can be NULL, if end_idx <= 0 and execution goes straight to ex:
- in xorriso/read_run.c, when catcontent is NULL, NULL is put in buf_pt and is passed to write() function
4) Missing initialization - in xorriso/iso_manip.c, own_link_stack isn't initialized, this matters in case that there was a problem while allocating the memory and execution continues to ex: where own_link_stack is freed; fix: initialize it to NULL (that the free function can handle)
And we have some improvements to make the code safer and cleaner:
1) Resource leaks - in xorriso/drive_mgt.c, dynamically allocated disc isn't freed in case that goto ex; is used; fix: move freeing after ex:
- in xorriso/emulators.c and xorriso/opts_i_o.c,arguments should be freed by Sfile_destroy_argv function
2) Overflow in strcpy - in multiple files, there are strcpys which copy dynamically allocated strings into static strings, it would be better to use strncpy to avoid overflow
3) Memcpy - in libisoburn/isofs_wrap.c, strncpy is used to copy fixed-size memory without ending null (Coverity reported it as a mistake), memcpy would be probably better in order to express the intent more clearly and to avoid false-positive warning in the future.
libisoburn_compilator_bitwise_op_labs.patch