informal code review of the libconf branch

[clepple]

Some of the warnings on the clang builders prompted me to take a closer look at the libconf branch.

• https://github.com/networkupstools/nut/blob/libconf/common/nutstream.cpp#L346

NutStream::status_t NutFile::putData(const std::string & data) has a comment that there is not a FILE* interface for writing arbitrary data. What about fwrite()?

• https://github.com/networkupstools/nut/blob/libconf/common/nutstream.cpp#L211

tempnam() should probably be replaced with mkstemp(). Then you won't need the hard-coded /var/tmp variable. (Some systems implement per-user $TMPDIR variables, which eliminate some corner cases and race conditions.)

• https://github.com/networkupstools/nut/blob/libconf/common/nutstream.cpp#L488

This expression won't do bit operations: ss << (packed && 0x000000ff) << ".";

Also, you should consider using the standard inet_?to? functions here, especially for the IPv6 address printer.

[EmilienKia]

I have started a review of this branch to intend to rework the branch to make it cleaner to read.

I want to:

• extract all work not in relation to libconf/nutconf in other branches (mainly mge-xml enhancements and AIX support)
• rework some commits:
  • split commits mixing libconf and nutconf in distinct ones
  • merging commits when some glitches occur
• split, if possible, nutconf tool code from libconf, in two distinct branches nutconf emerging from libconf
• rebase all this f###ing base of code more up to date.

Based on that, I think some work on libconf should be done:

• Revamp code of parser/writer on modules with coherent name scheme
• Really use data source to parse (and not dump it in string as now)
• Write more doc. IMHO the doc (mine in particular) is not really understandable and usable.
• And so much more ...

[EmilienKia]

First of all, I think Vasek can precise what he has done for mge-xml and AIX.

Have the code been merged (or picked) to master ? as master looks like having similar code ?

[clepple]

Fred opened a few pull requests for Vasek's work. Some of the commit messages were not very specific, so they were cherry-picked and reworded (rather than merged).

AIX compilation fixes: #58 mge-xml: #59

I am not sure about some of the AIX packaging improvements.

If you do an interactive rebase onto master, then you probably want to skip d2e1bb07, 704d763d, and 72d3fd12 from the mge-xml fixes. Unfortunately, there may be some other merge conflicts if Git doesn't pick up the rename of configure.in to configure.ac.

[EmilienKia]

Thanks Charles, I will look at them.

[EmilienKia]

After investigations, I can confirm that commit 7b16fac closing bug #59 is about the squashing of commits 6ffc259, e54ce1c, 1695533, d2e1bb0, 704d763 and 72d3fd1. In consequence, if the libconf branch is reworked, these commits can be removed.