On 11/25/2016 11:18 AM, Jan Beulich wrote:
On 25.11.16 at 09:03, <andr2...@gmail.com> wrote:
+#ifndef __XEN_PUBLIC_IO_XENSND_H__
+#define __XEN_PUBLIC_IO_XENSND_H__
+
+#include <xen/interface/io/ring.h>
+#include <xen/interface/grant_table.h>
Along with the target tree (and hence path) change, these also want
to become ""-style #include-s.
done
+struct xensnd_open_req {
+ uint32_t pcm_rate; /* in Hz */
+ uint8_t pcm_format;
+ uint8_t pcm_channels;
+ uint16_t __reserved0;
No double underscores please at the beginning of these names;
preferably none at all (as field names may collide with file scope
object-like macros).
changed __reserver0 to reserved
+struct xensnd_page_directory {
+ grant_ref_t gref_dir_next_page;
+ uint32_t num_grefs;
+ grant_ref_t gref[0];
This is not allowed in C89, and I think not even in newer version (i.e.
it's a GNU extension). There are a couple of places where we already
deal with similar constructs, so please take those into consideration
when dealing with the problem here.
C99 says (6.2.5.20):
" An array type describes a contiguously allocated nonempty set of
objects with a particular member object type,"
will change to *gref[1] /* Variable length */* as it is done in fsif.h
+struct xensnd_close_req {
+ /* place holder, remove if changing the structure (C89 concern) */
+ uint8_t __placeholder;
+};
changed __placeholder to placeholder
The comment ought to be correct - did you verify this is permitted
in e.g. C99? I don't think it is, and I've referred to C89 only to give
you an understanding with what standard the header as a whole
is expected to comply (i.e. to avoid you using C99 constructs
elsewhere).
you are right, C99 says ( 6.2.5.20):
" A structure type describes a sequentially allocated *nonempty* set of
member objects"
C11 doesn't say anything that it is allowed
So, I will change in the comments *C89 concern* to *(C99 6.2.5.20)*
+union xensnd_req {
+ struct {
+ uint16_t id;
+ uint8_t operation;
+ uint8_t stream_idx;
+ uint32_t padding;
+ union {
+ struct xensnd_open_req open;
+ struct xensnd_close_req close;
+ struct xensnd_write_req write;
+ struct xensnd_read_req read;
+ struct xensnd_get_vol_req get_vol;
+ struct xensnd_set_vol_req set_vol;
+ struct xensnd_mute_req mute;
+ struct xensnd_unmute_req unmute;
+ } op;
+ } data;
+ uint8_t raw[64];
+};
Hmm, you've indeed changed the way you indent, but there are
more style issues here: The inner union's members aren't all
indented equally, and (perhaps as a result) the closing braces
are indented too much.
done
+union xensnd_resp {
+ struct {
+ uint16_t id;
+ uint8_t operation;
+ uint8_t stream_idx;
+ int8_t status;
+ } data;
This lacks explicit tail padding.
done, added *uint8_t padding[3];*
Jan
Thank you,
Oleksandr
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
