Hello Wouter,
It was <2024-02-20 wto 14:06>, when Wouter Verhelst wrote:
> On Thu, Feb 01, 2024 at 08:30:46PM +0100, Lukasz Stelmach wrote:
>> It was <2024-01-31 śro 21:14>, when Wouter Verhelst wrote:
>>> On Mon, Jan 22, 2024 at 12:52:23PM +0100, Lukasz Stelmach wrote:
>>>>
>>>> Last year we decided to include nbd package in Tizen. Every package we
>>>> use gets a periodic treatment with static analysis tools (Coverity,
>>>> SVACE). There were some problems detected in nbd and we've developed a
>>>> number of patches[1] to fix them. Please take a look at them as well as
>>>> the eariler patch[1] I posted.
>>>>
>>>> [1] https://git.tizen.org/cgit/platform/upstream/nbd/log/?h=tizen&id=2c54e25b0cea8d30f7958fa2d17c67b91867aff6
>>>
>>> Is this available in some public repository that I can just pull from? I
>>> tried https://git.tizen.org/platform/upstream/nbd, which seemed the
>>> most obvious, but that didn't work.
>>
>> That's
>>
>> git fetch https://git.tizen.org/cgit/platform/upstream/nbd 2c54e25b0cea8d30f7958fa2d17c67b91867aff6
>>
>> with "/cgit/" between ".org" and "platform".
>>
>>> I didn't like this because there are some autoconf variables that (by
>>> default) have other variables in them, in ways that expose them as shell
>>> variables; e.g., the default setting for sysconfdir is "${prefix}/etc",
>>> which means that if you run "./configure" with no arguments,
>>> nbd-server(5) will mention that the configuration file is
>>> "${prefix}/etc/nbd-server/config", which I think is a bit ugly.
>>
>> It is indeed, but I believe it's not the case.
>>
>>> That's why I did it that way.
>>>
>>> I don't think your version does that? But I could be wrong, of course;
>>> in which case, please explain :)
>>
>> I wrote the patch some time ago (-; but I believe I haven't seen such
>> effect. Let me try. I run:
>>
>> git checkout e0a7534c74e7e21fe0be2ad55e4cbe35cef21a07
>> ../autogen.sh
>> configure
>> grep nbd-server/config -- man/nbd-server.5.sgml
>>
>> <!ENTITY dhpackage "/usr/local/etc/nbd-server/config">
>> <filename>/usr/local/etc/nbd-server/config</filename> is the default
>>
>> Next
>>
>> make -C man/ nbd-server.5
>> grep nbd-server/config -- man/nbd-server.5
>>
>> /usr/local/etc/nbd-server/config \- configuration file for nbd-server
>> \fB/usr/local/etc/nbd-server/config \fR
>> \fI/usr/local/etc/nbd-server/config\fR is the default
>>
>> If there are any problems, would you mind, responding to that patch and
>> pointing them out? I'd like to fix them.
>
> I finally had time to have a look at your patches. Unfortunately, they
> break the test suite; if you run "make check", the result is fairly
> depressing :)
>
> Can you have a look at what's going wrong?
I "fixed" one too many memory leaks reported by Coverity. I'd reverted[1]
it and it helped
--8<---------------cut here---------------start------------->8---
$ make check
make check-recursive
[…]
==================
All 5 tests passed
==================
[…]
./inetd
Error: inetd mode not supported without syslog support
SKIP: inetd
[…]
====================
All 18 tests passed
(1 test was not run)
====================
--8<---------------cut here---------------end--------------->8---
>> PS. I received thee more bug reports to handle. I'll review them when I
>> get back from FOSDEM, so you don't need to hurry with these.
Below the revert there are two more commits ("Fix out-of-bounds access")
I created after 2c54e25b0c we talked about last time.
[1] https://git.tizen.org/cgit/platform/upstream/nbd/log/?id=9e25a075173345410de58ac9406a464fce929531
Kind regards,
--
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics
Attachment:
signature.asc
Description: PGP signature