Re: [Bug-xorriso] Fixes and improvements for libisoburn

[Thomas Schmitt]

Hi,

Eliska Svobodova wrote:
> > > 2) Absolute value - you are using abs() to calculate the
> > >    absolute value of r1count and r2count, [...]
> > >    fix: use labs()

I wrote:
> > They are off_t rather than long int. The whole ado with (double)
> > is because there is no printf formatter for off_t

> Pavel Cahyna suggested you use intmax_t in place of
> double because printf has a conversion for it

Well, double is there since Kernighan & Ritchie.
I would agree to modernizing the code to this century. But an ISO 9660
filesystem with block size 2048 can only address by 11 + 32 = 43 bits
because blocks are addressed by 32 bit.
double can handle this without loss of exactness.


> > > And we have some improvements [...]
> > > 1) Resource leaks - [...]

> > So how come that the analizer reported two false positives and
> > missed one true positive ?

> All reports about resource leaks are from Coverity.
> libisoburn-1.4.8/xorriso/emulators.c:1033: alloc_arg: "Xorriso_read_lines"
> allocates memory that is stored into "argv".
> [...]
> With Pavel Cahyna, we might also have discovered the reason why Coverity
> reported those false-positive resource leaks. It cannot understand flags,

Somewhat astounding, given that it looked into the called function for
memory allocators. The memory releaser is the first operation in there.
Only then - conditionally - comes new allocation.

But the marks in the .html attachments are unambiguous. So there was
no other valid aspect in the complaint which would need to be investigated.


> > > 2) Overflow in strcpy - [...]

> I am glad that at least one message was right. I am sorry I bothered you
> with so many false-positives, I should have checked them more closely. I
> just thought that additional control wouldn't mind.

Last century coding style can be confusing to this century's automats.
It was worth the effort to check. Even if not a single bug had been found.
Memory leaks invite the OOM killer.


> (this is my first attempt in contributing).

I perceived it as helpful. Thank you.


Some nitpicking, though, about the HTML attachments:

> 57opts_i_o.c.html  56opts_i_o.c.html  55emulators.c.html

These were helpful to understand the complaints about Xorriso_read_lines().
But it would have saved bandwidth, storage, and scrolling effort if most
of the lines before and after the annotations had been cut away.
(Well, ok, i could make smaller source files ...)


Have a nice day :)

Thomas