Re: Fixes after static code analysis

[Wouter Verhelst <w@uter.be>]

Hi Lukasz,

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?

Note: if you need this the "simple_test" script which sets up nbd-server
and tries to run the tests against it has the ability to set up an
environment for you to run things inside the debugger easily: just run 

bash -x ./simple_test <test name> <any second argument>

and it will invoke that test, leaving the configuration around so you
can then run nbd-server manually (as the script will have just shown
you) and do things inside a debugger.

Thanks,

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:
> >> Hi,
> >> 
> >> 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".
> 
> >> [2] https://lists.debian.org/nbd/2023/08/msg00046.html
> >
> > I looked at that when you sent it, didn't like it, was going to reply,
> > but then apparently forgot to do that. Sorry about that.
> 
> NP.
> 
> > 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.
> 
> 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.
> 
> PPS. If you happen to be around, Wouter, and would like to meet, try
> reaching me via Matrix @steelman:matrix.org
> -- 
> Łukasz Stelmach
> Samsung R&D Institute Poland
> Samsung Electronics



-- 
     w@uter.{be,co.za}
wouter@{grep.be,fosdem.org,debian.org}

I will have a Tin-Actinium-Potassium mixture, thanks.