Re: [libcoap-developers] New topic-branch 'dtls' for testing and evaluation available

[Carsten S. <c.s...@t-...>]

Hello Olaf,

Am 29.03.2016 um 16:30 schrieb Olaf Bergmann:
> Hi all,
> 
> it took a long time, but finally, I have come up with an improved update
> of my previous DTLS-integration branch*) I have pushed it to github as a
> topic branch for test and evaluation, any feedback is appreciated. The
> latest development version of tinydtls is included as a submodule for
> convenience hence you should clone this branch using
> 
>   git clone --recursive -b dtls https://github.com/obgm/libcoap.git

I haven't done much testing until now due a lack of time and usable
devices. ;) So currently I'm looking more on the technical source and
formating within these changes.

>From a quick check ...
> carsten@x201s:~/gitprojects/libcoap [dtls] $ ./autogen.sh
> ...
> carsten@x201s:~/gitprojects/libcoap [dtls] $ ./configure
> ...
> carsten@x201s:~/gitprojects/libcoap [dtls] $ make
> Makefile:699: src/platform/posix/.deps/libcoap_1_la-coap_io.Plo: No such file or directory
> Makefile:700: src/platform/posix/.deps/libcoap_1_la-prng.Plo: No such file or directory
> Makefile:701: src/platform/posix/.deps/libcoap_1_la-run.Plo: No such file or directory
> make: *** No rule to make target 'src/platform/posix/.deps/libcoap_1_la-run.Plo'.  Stop.

There seems something missed for the autotool magic. Probably you need a
Makefile.am in src/platform/posix/ and expand AC_CONFIG_FILES in
configure.ac?
The variable $(subdirs) seems never defined, so automake will not jump
into the desired subdirectory. I suggest to use a upper case letter
naming here. Something like 'PLATFORM_SUBDIR' which I would use as the
expected content would be visible.

I'm fine with the splitting of the source into the platform related
files and structures. This increases the readability and maintenance of
the code in the right way.

Could you please look at trailing whitespaces and some old tabs
insertion within moved or modified code in the dtls branch and rebase
the commits if necessary? For example commit
ff74ebe176443f9b0e5e2b6b1689368782572dfc (coap_io.c: Moved to
platform/posix/) is renaming src/coap_io.c into
src/platform/posix/coap_io.c. I would consider to go through the final
file(s) an adjust the formating to the current schema.
This would prevent commits which probably only reformat the existing code.

Small nitpick on 1c99d22d899b30330bb1e77ee325f78e3e560b89 (dtls: added
initial boilerplate for dtls support)

> +# __dtls__
^^^ should use the small explanation above like on other configure options

> +# configure options
> +# __dtls__

[snip]
> Please note that this code is still very alpha. The coap-server example
> uses the tinydtls default test credentials (Client_identity:secretPSK).
> The coap-client example has two new command line options '-u' and '-k',
> respectively, to pass the user name and the corresponding key. (This has
> obviously room for improvement as this interface only allows printable
> characters to be provided.)
> 
> *) Rationale: Last year, there have been some activities to refactor the
>    platform-specific code and to integrate OpenSSL. I had put my
>    tinydtls integration on hold for some time to wait for the results of
>    those actitivies. As these seem to have stuck somewhere in the
>    process, I have decided to resume my work because I really, really,
>    REALLY want to have a next libcoap release with DTLS support as soon
>    as possible.

I'm in finally preparations for a upload of the current state in the
develop branch into Debian unstable. As libcoap is already in
experimental available there are no show stoppers to expect. The freeze
for the stretch release will be on 5th November 2016, so yeah, if 4.1.2
or a 4.2 release should be shipped in Stretch libcoap should be prepared. :)

I started some time ago a man page (7) for the library itself which
should cover up the usage and explanations about how to use the symbols
and in which order. Any help in getting this done is much appreciated.

-- 
Regards
Carsten Schoenert