[spice-gtk,v4,01/13] cd-sharing: add cd-device header file

Submitted by Yuri Benditovich on Sept. 17, 2018, 1:22 p.m.

Details

Message ID 1537190583-4576-2-git-send-email-yuri.benditovich@daynix.com
State New
Headers show
Series "CD sharing feature" ( rev: 4 ) in Spice

Not browsing as part of any series.

Commit Message

Yuri Benditovich Sept. 17, 2018, 1:22 p.m.
Contains definitions for system-dependent operations
related to local cd devices and files

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 src/cd-device.h | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 src/cd-device.h

Patch hide | download patch | download mbox

diff --git a/src/cd-device.h b/src/cd-device.h
new file mode 100644
index 0000000..050c7a1
--- /dev/null
+++ b/src/cd-device.h
@@ -0,0 +1,40 @@ 
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+Copyright (C) 2018 Red Hat, Inc.
+
+Red Hat Authors:
+Yuri Benditovich<ybendito@redhat.com>
+
+This library is free software; you can redistribute it and/or
+modify it under the terms of the GNU Lesser General Public
+License as published by the Free Software Foundation; either
+version 2.1 of the License, or (at your option) any later version.
+
+This library is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public
+License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef __CD_DEVICE_H__
+#define __CD_DEVICE_H__
+
+typedef struct _SpiceCdLU
+{
+    char *filename;
+    GFile *file_object;
+    GFileInputStream *stream;
+    uint64_t size;
+    uint32_t blockSize;
+    uint32_t loaded : 1;
+    uint32_t device : 1;
+} SpiceCdLU;
+
+int cd_device_open_stream(SpiceCdLU *unit, const char *filename);
+int cd_device_load(SpiceCdLU *unit, gboolean load);
+int cd_device_check(SpiceCdLU *unit);
+
+#endif

Comments

> 
> Contains definitions for system-dependent operations
> related to local cd devices and files
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  src/cd-device.h | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 src/cd-device.h
> 
> diff --git a/src/cd-device.h b/src/cd-device.h
> new file mode 100644
> index 0000000..050c7a1
> --- /dev/null
> +++ b/src/cd-device.h
> @@ -0,0 +1,40 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> +Copyright (C) 2018 Red Hat, Inc.
> +
> +Red Hat Authors:
> +Yuri Benditovich<ybendito@redhat.com>
> +
> +This library is free software; you can redistribute it and/or
> +modify it under the terms of the GNU Lesser General Public
> +License as published by the Free Software Foundation; either
> +version 2.1 of the License, or (at your option) any later version.
> +
> +This library is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +Lesser General Public License for more details.
> +
> +You should have received a copy of the GNU Lesser General Public
> +License along with this library; if not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +#ifndef __CD_DEVICE_H__
> +#define __CD_DEVICE_H__
> +
> +typedef struct _SpiceCdLU
> +{
> +    char *filename;
> +    GFile *file_object;
> +    GFileInputStream *stream;
> +    uint64_t size;
> +    uint32_t blockSize;
> +    uint32_t loaded : 1;
> +    uint32_t device : 1;
> +} SpiceCdLU;

Why not SpiceCdDevice? In neither APIs, title or commit message there's
no explanation for the "LU", Logical Unit?
In C99 identifiers starting with double underscore or underscore and
capital letter are reserved, do you need C89 compatibility?

> +
> +int cd_device_open_stream(SpiceCdLU *unit, const char *filename);
> +int cd_device_load(SpiceCdLU *unit, gboolean load);
> +int cd_device_check(SpiceCdLU *unit);

Maybe some small comment. What the functions return? <0 error, >=0 success?
"check" I assume is checking disk presence.
Why you have an open but not a close? Is it implicit?
How to initialize SpiceCdLU? memset to 0?

> +
> +#endif

Frediano
On Mon, Sep 17, 2018 at 9:37 PM, Frediano Ziglio <fziglio@redhat.com> wrote:

> >
> > Contains definitions for system-dependent operations
> > related to local cd devices and files
> >
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >  src/cd-device.h | 40 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >  create mode 100644 src/cd-device.h
> >
> > diff --git a/src/cd-device.h b/src/cd-device.h
> > new file mode 100644
> > index 0000000..050c7a1
> > --- /dev/null
> > +++ b/src/cd-device.h
> > @@ -0,0 +1,40 @@
> > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > +/*
> > +Copyright (C) 2018 Red Hat, Inc.
> > +
> > +Red Hat Authors:
> > +Yuri Benditovich<ybendito@redhat.com>
> > +
> > +This library is free software; you can redistribute it and/or
> > +modify it under the terms of the GNU Lesser General Public
> > +License as published by the Free Software Foundation; either
> > +version 2.1 of the License, or (at your option) any later version.
> > +
> > +This library is distributed in the hope that it will be useful,
> > +but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +Lesser General Public License for more details.
> > +
> > +You should have received a copy of the GNU Lesser General Public
> > +License along with this library; if not, see <
> http://www.gnu.org/licenses/>.
> > +*/
> > +
> > +#ifndef __CD_DEVICE_H__
> > +#define __CD_DEVICE_H__
> > +
> > +typedef struct _SpiceCdLU
> > +{
> > +    char *filename;
> > +    GFile *file_object;
> > +    GFileInputStream *stream;
> > +    uint64_t size;
> > +    uint32_t blockSize;
> > +    uint32_t loaded : 1;
> > +    uint32_t device : 1;
> > +} SpiceCdLU;
>
> Why not SpiceCdDevice? In neither APIs, title or commit message there's
> no explanation for the "LU", Logical Unit?
>

This is logical unit, when CD device contains one or more units


> In C99 identifiers starting with double underscore or underscore and
> capital letter are reserved, do you need C89 compatibility?
>

To be fixed on next round


>
> > +
> > +int cd_device_open_stream(SpiceCdLU *unit, const char *filename);
> > +int cd_device_load(SpiceCdLU *unit, gboolean load);
> > +int cd_device_check(SpiceCdLU *unit);
>
> Maybe some small comment. What the functions return? <0 error, >=0 success?
>

Yes, as usually should be.


> "check" I assume is checking disk presence.
> Why you have an open but not a close? Is it implicit?
>

close does not require any platform-dependent operations and is on common
part


> How to initialize SpiceCdLU? memset to 0?
>

Initialized outside, according to parameters, defaults to zero.


>
> > +
> > +#endif
>
> Frediano
>