Re: [PATCH] Aseqnet, no nagle and dual stack

[Takashi Iwai <tiwai@suse.de>]

On Tue, 23 Apr 2019 20:25:58 +0200,
Rob van der Putten wrote:
On Tue, 23 Apr 2019 20:25:58 +0200,
Rob van der Putten wrote:
> 
> Hi there
> 
> 
> Not an ALSA source patch, but a patch for an ALSA related util.
> Aseqnet sends ALSA sound_seq MIDI over TCP/IP. The patch below
> disables nagle, enables quickack and makes aseqnet dual-stack.

Thanks for the patch.  Could you repost with a proper patch change log
and your Signed-off-by line so that one can apply to git repo?

About the changes:
> --- aseqnet.c.bak	2012-01-25 10:43:38.000000000 +0100
> +++ aseqnet.c	2017-08-26 14:17:58.261868853 +0200
> @@ -3,6 +3,8 @@
>   *   ver.0.1
>   *
>   * Copyright (C) 1999-2000 Takashi Iwai
> + * Modified by Rob van der Putten, Leiden, Holland,
> + * rob at sput dot nl.

We don't need to add each change in the source like that as all
tracked in git.

> @@ -15,18 +17,21 @@
>   *
>   */
> 
> +#include <alsa/asoundlib.h>
> +#include <arpa/inet.h>
> +#include <assert.h>
> +#include <ctype.h>
> +#include <getopt.h>
> +#include <locale.h>
> +#include <netdb.h>
> +#include <netinet/in.h>
> +#include <netinet/tcp.h>
> +#include <signal.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <ctype.h>
>  #include <string.h>
> -#include <netinet/in.h>
>  #include <sys/socket.h>
> -#include <netdb.h>
> -#include <locale.h>
> -#include <alsa/asoundlib.h>
> -#include <getopt.h>
> -#include <signal.h>
> -#include <assert.h>
> +#include <sys/types.h>
>  #include "aconfig.h"
>  #include "gettext.h"

Why these large rearrangement of include files?  If it must be
inevitably done, please describe the reason in the changelog, too.

> @@ -327,17 +332,24 @@
>   */
>  static void init_server(int port)
>  {
> +	/*
> +	*       RvdP, changed to support IPv6
> +	*       Dual stack only!
> +	*/

Wouldn't it potentially break things?  IMO, it's better to keep the
old behavior (ipv4-only), at least, with an option.
Also, drop your initials in the commit, it's rather superfluous.


thanks,

Takashi