[-,alsa-lib,1/1] Introduce snd_seq_client_info_get_card_number, for getting the card number of a seq client on recent kernels

Commit Message

Adam Goode June 5, 2015, 1:05 a.m. UTC

Signed-off-by: Adam Goode <[e-mail]>

Comments

Clemens Ladisch June 5, 2015, 6:50 a.m. UTC | #1

Adam Goode wrote:
>  /**
> + * \brief Get the card number of a client_info container
> + * \param info client_info container
> + * \return the card number, -1 if no card associated with this client, or -ENOSYS if the
> + *         kernel does not support reporting this field
> + */

-1 is used as an out-of-range value when enumerating cards.  However,
I don't think this convention can be applied here, because the return
value is also used for error codes, and -1 would correspond to -EPERM.

Squashing all error codes into -1 is not a good idea, so I think the
value -1 should be converted into an appropriate error code, probably
-ENXIO.


Regards,
Clemens

Takashi Iwai June 10, 2015, 10:35 a.m. UTC | #2

At Fri, 05 Jun 2015 08:50:45 +0200,
Clemens Ladisch wrote:
> 
> Adam Goode wrote:
> >  /**
> > + * \brief Get the card number of a client_info container
> > + * \param info client_info container
> > + * \return the card number, -1 if no card associated with this client, or -ENOSYS if the
> > + *         kernel does not support reporting this field
> > + */
> 
> -1 is used as an out-of-range value when enumerating cards.  However,
> I don't think this convention can be applied here, because the return
> value is also used for error codes, and -1 would correspond to -EPERM.
> 
> Squashing all error codes into -1 is not a good idea, so I think the
> value -1 should be converted into an appropriate error code, probably
> -ENXIO.

Agreed.

Also, please don't include irrelevant changes in configure.ac.


thanks,

Takashi

Adam Goode June 10, 2015, 5:09 p.m. UTC | #3

On Wed, Jun 10, 2015 at 6:35 AM, Takashi Iwai <[e-mail]> wrote:

> At Fri, 05 Jun 2015 08:50:45 +0200,
> Clemens Ladisch wrote:
> >
> > Adam Goode wrote:
> > >  /**
> > > + * \brief Get the card number of a client_info container
> > > + * \param info client_info container
> > > + * \return the card number, -1 if no card associated with this
> client, or -ENOSYS if the
> > > + *         kernel does not support reporting this field
> > > + */
> >
> > -1 is used as an out-of-range value when enumerating cards.  However,
> > I don't think this convention can be applied here, because the return
> > value is also used for error codes, and -1 would correspond to -EPERM.
> >
> > Squashing all error codes into -1 is not a good idea, so I think the
> > value -1 should be converted into an appropriate error code, probably
> > -ENXIO.
>
> Agreed.
>
> Also, please don't include irrelevant changes in configure.ac.
>
>
> thanks,
>
> Takashi
>


Thanks for the feedback, I should have time this week to get back to you.


Adam

Patch

diff --git a/configure.ac b/configure.ac
index 9621d4e..01bb0fb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -14,7 +14,7 @@  dnl remove API = c+1:0:0
 dnl *************************************************
 AC_CANONICAL_HOST
 AM_INIT_AUTOMAKE
-eval LIBTOOL_VERSION_INFO="2:0:0"
+eval LIBTOOL_VERSION_INFO="3:0:1"
 dnl *************************************************
 AM_CONDITIONAL([INSTALL_M4], [test -n "${ACLOCAL}"])
 
diff --git a/include/seq.h b/include/seq.h
index 9576822..e0f9b78 100644
--- a/include/seq.h
+++ b/include/seq.h
@@ -146,6 +146,7 @@  int snd_seq_client_info_get_error_bounce(const snd_seq_client_info_t *info);
 const unsigned char *snd_seq_client_info_get_event_filter(const snd_seq_client_info_t *info);
 int snd_seq_client_info_get_num_ports(const snd_seq_client_info_t *info);
 int snd_seq_client_info_get_event_lost(const snd_seq_client_info_t *info);
+int snd_seq_client_info_get_card_number(const snd_seq_client_info_t *info);
 
 void snd_seq_client_info_set_client(snd_seq_client_info_t *info, int client);
 void snd_seq_client_info_set_name(snd_seq_client_info_t *info, const char *name);
diff --git a/include/sound/asequencer.h b/include/sound/asequencer.h
index 09c8a00..c2a659b 100644
--- a/include/sound/asequencer.h
+++ b/include/sound/asequencer.h
@@ -22,9 +22,10 @@ 
 #ifndef _UAPI__SOUND_ASEQUENCER_H
 #define _UAPI__SOUND_ASEQUENCER_H
 
+#include <sound/asound.h>
 
 /** version of the sequencer */
-#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION (1, 0, 1)
+#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION (1, 0, 2)
 
 /**
  * definition of sequencer event types
@@ -356,7 +357,8 @@  struct snd_seq_client_info {
 	unsigned char event_filter[32];	/* event filter bitmap */
 	int num_ports;			/* RO: number of ports */
 	int event_lost;			/* number of lost events */
-	char reserved[64];		/* for future use */
+	int card_number;		/* RO: card number, or -1 if no card. Added in protocol version 1.0.2 */
+	char reserved[60];		/* for future use */
 };
 
 
diff --git a/src/seq/seq.c b/src/seq/seq.c
index 620ca3f..bc39046 100644
--- a/src/seq/seq.c
+++ b/src/seq/seq.c
@@ -1651,6 +1651,18 @@  int snd_seq_client_info_get_event_lost(const snd_seq_client_info_t *info)
 }
 
 /**
+ * \brief Get the card number of a client_info container
+ * \param info client_info container
+ * \return the card number, -1 if no card associated with this client, or -ENOSYS if the
+ *         kernel does not support reporting this field
+ */
+int snd_seq_client_info_get_card_number(const snd_seq_client_info_t *info)
+{
+	assert(info);
+	return info->card_number;
+}
+
+/**
  * \brief Set the client id of a client_info container
  * \param info client_info container
  * \param client client id
diff --git a/src/seq/seq_hw.c b/src/seq/seq_hw.c
index d033367..8502d63 100644
--- a/src/seq/seq_hw.c
+++ b/src/seq/seq_hw.c
@@ -32,10 +32,11 @@  const char *_snd_module_seq_hw = "";
 #ifndef DOC_HIDDEN
 #define SNDRV_FILE_SEQ		ALSA_DEVICE_DIRECTORY "seq"
 #define SNDRV_FILE_ALOADSEQ	ALOAD_DEVICE_DIRECTORY "aloadSEQ"
-#define SNDRV_SEQ_VERSION_MAX	SNDRV_PROTOCOL_VERSION(1, 0, 1)
+#define SNDRV_SEQ_VERSION_MAX	SNDRV_PROTOCOL_VERSION(1, 0, 2)
 
 typedef struct {
 	int fd;
+	int version;
 } snd_seq_hw_t;
 #endif /* DOC_HIDDEN */
 
@@ -100,6 +101,10 @@  static int snd_seq_hw_get_client_info(snd_seq_t *seq, snd_seq_client_info_t * in
 		/*SYSERR("SNDRV_SEQ_IOCTL_GET_CLIENT_INFO failed");*/
 		return -errno;
 	}
+	/* The card_number field was added in protocol 1.0.2, so set to -ENOSYS if older. */
+	if (hw->version < SNDRV_PROTOCOL_VERSION(1, 0, 2)) {
+		info->card_number = -ENOSYS;
+	}
 	return 0;
 }
 
@@ -368,6 +373,10 @@  static int snd_seq_hw_query_next_client(snd_seq_t *seq, snd_seq_client_info_t *i
 		/*SYSERR("SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT failed");*/
 		return -errno;
 	}
+	/* The card_number field was added in protocol 1.0.2, so set to -ENOSYS if older. */
+	if (hw->version < SNDRV_PROTOCOL_VERSION(1, 0, 2)) {
+		info->card_number = -ENOSYS;
+	}
 	return 0;
 }
 
@@ -480,6 +489,7 @@  int snd_seq_hw_open(snd_seq_t **handle, const char *name, int streams, int mode)
 		return -ENOMEM;
 	}
 	hw->fd = fd;
+	hw->version = ver;
 	if (streams & SND_SEQ_OPEN_OUTPUT) {
 		seq->obuf = (char *) malloc(seq->obufsize = SND_SEQ_OBUF_SIZE);
 		if (!seq->obuf) {
diff --git a/test/seq.c b/test/seq.c
index 34b000f..a4e5f1f 100644
--- a/test/seq.c
+++ b/test/seq.c
@@ -111,7 +111,7 @@  void show_port_info(snd_seq_t *handle, int client, int port)
 
 void show_client_info(snd_seq_t *handle, int client)
 {
-	int err, idx, min, max;
+	int err, idx, min, max, card_number;
 	snd_seq_client_info_t *info;
 
 	snd_seq_client_info_alloca(&info);
@@ -124,11 +124,14 @@  void show_client_info(snd_seq_t *handle, int client)
 			fprintf(stderr, "Client %i info error: %s\n", idx, snd_strerror(err));
 			exit(0);
 		}
+		card_number = snd_seq_client_info_get_card_number(info);
 		printf("Client %i info\n", idx);
 		if (verbose)
 			printf("  Client        : %i\n", snd_seq_client_info_get_client(info));
 		printf("  Type          : %s\n", snd_seq_client_info_get_type(info) == SND_SEQ_KERNEL_CLIENT ? "kernel" : "user");
 		printf("  Name          : %s\n", snd_seq_client_info_get_name(info));
+		if (card_number != -ENOSYS)
+			printf("  Card Number   : %i\n", card_number);
 	}
 }