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

Frediano Ziglio fziglio at redhat.com
Mon Sep 17 18:37:51 UTC 2018


> 
> Contains definitions for system-dependent operations
> related to local cd devices and files
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich at 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 at 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


More information about the Spice-devel mailing list