[Spice-devel] [PATCH spice-server v6 00/10] SASL code improvements

Christophe Fergeau cfergeau at redhat.com
Tue Jan 9 16:54:43 UTC 2018


Hey,

There seems to have 2 series mixed together, one with 9 patches, the
other with 10 patches, and with some identical patches between the 2.
I looked at the branch from your repository, and the series looks good
to me. Did not look carefully at the test code this time, but this can
always be improved if needed.
For the series,
Acked-by: Christophe Fergeau <cfergeau at redhat.com>

On Tue, Jan 09, 2018 at 07:44:55AM +0000, Frediano Ziglio wrote:
> SASL authentication code were a bit dodgy to see.
> The series have 2 parts:
> - a test for SASL code;
> - some code refactory.
> 
> Test is not that straightforward, maybe some code should be put in
> different files. On the other end I don't think the code that is
> mocking SASL will be reused in other tests.
> The refactory is mainly put all the authentication logic in one file
> (red-stream.c) instead of having half in reds.c and the other half in
> red-stream.c. This allows to present a single function instead of one
> for each authentication step.
> 
> Changes since v5:
> - make the client emulator more robust making possible to run on
>   Valgrind which causes some timing and syscall difference
>   (disconnection is detected earlier causing a write after server
>   close the connection to fail).
> 
> Changes since v4:
> - test cases where SASL APIs reply too large packets;
> - test the way a failure is reported from server.
> 
> Changes since v3:
> - tested with a MIPS machine (quite hard to do, I should write how to
>   do it!);
> - change way to iterate tests;
> - lot of comments;
> - some style changes;
> - fixed a regression for big endian machines.
> 
> Changes since v2:
> - removed patch to keep mechlist without copy, not documented really
>   fine, safer to copy it.
> 
> Changes since v1:
> - rebase on master;
> - merged some patches;
> - improved and fixed some comments;
> - minor styles.
> 
> Frediano Ziglio (10):
>   test-sasl: Initial SASL test
>   test-sasl: Add code to mocking functions to test state
>   test-sasl: Base test, connect using SASL
>   test-sasl: Add tests for different mechanism names
>   test-sasl: Add tests for different failures and cases
>   test-sasl: Test how to server reports the failure
>   Handle SASL initialisation mainly in red-stream.c
>   red-stream: Handle properly endianness in SASL code
>   red-stream: Unify start and step passes
>   red-stream: Encapsulate all authentication state in RedSASLAuth
> 
>  configure.ac             |   1 +
>  server/red-stream.c      | 330 ++++++++++-------------
>  server/red-stream.h      |  11 +-
>  server/reds.c            | 119 +--------
>  server/tests/.gitignore  |   1 +
>  server/tests/Makefile.am |   4 +
>  server/tests/test-sasl.c | 665 +++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 825 insertions(+), 306 deletions(-)
>  create mode 100644 server/tests/test-sasl.c
> 
> -- 
> 2.14.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180109/2312abde/attachment.sig>


More information about the Spice-devel mailing list