persistent property store - first try

[Joe Shaw]

On Fri, 2004-06-11 at 00:06 +0200, Kay Sievers wrote:
> I've added a persistence attribute to the property and added a 3 line
> hack to hald_dbus to test it. All properties set through the libhal-api
> are persistent at the moment.

Nice!  I think the hack is fine until we determine which properties
exactly to persist.

> Thanks for your comments, Joe. It's my first contact with glib, so I
> really appreciate it.

Happy to help.  

One more comment, I missed it in my first email... :P

> --- /dev/null	2004-02-23 22:02:56.000000000 +0100
> +++ hal.kay/hal/hald/pstore.c	2004-06-10 22:01:28.883678048 +0200
> @@ -0,0 +1,368 @@
> +/***************************************************************************
> + * CVSID: $Id: $
> + *
> + * pstore.c : persistent property store on disk
> + *
> + * Copyright (C) 2004 Kay Sievers, <kay.sievers at vrfy.org>
> + *
> + * Licensed under the Academic Free License version 2.0
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + **************************************************************************/
> +
> +#ifdef HAVE_CONFIG_H
> +#  include <config.h>
> +#endif
> +
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <time.h>
> +#include <sys/stat.h>
> +#include <glib.h>
> +
> +#include "logger.h"
> +#include "pstore.h"
> +
> +#define PSTR		"String:"
> +#define PINT32		"Int32:"
> +#define PBOOL		"Bool:"
> +#define PDOUBLE		"Double:"
> +#define UDI_STRIP	"/org/freedesktop/Hal"
> +
> +static char *pstore_uuid = NULL;
> +
> +struct _HalPStore {
> +	char *path;
> +	char *uuid;
> +};
> +
> +/** Create relative or absolute path.
> + *
> + */
> +static int
> +create_path(const char *path)
> +{
> +	char *p;
> +	char *pos;
> +	struct stat stats;
> +
> +	if (stat (path, &stats) == 0)
> +		return 0;
> +
> +	p = g_strdup (path);
> +	pos = strrchr (p, '/');
> +	if (pos != NULL && pos != p) {
> +		pos[0] = '\0';
> +		create_path (p);
> +	}
> +	g_free (p);
> +
> +	return mkdir(path, 0755);
> +}
> +
> +/** Return absolute filename with simplified udi string.
> + *
> + */
> +static char
> +*build_path (HalPStore *pstore,
> +	     const char *udi, const char *file)
> +{
> +	char *path;
> +	const char *udi_s;
> +
> +	udi_s = udi;
> +	/* strip namespace part of udi string */
> +	if (udi != NULL &&
> +	    g_ascii_strncasecmp(udi, UDI_STRIP, sizeof(UDI_STRIP)-1) == 0)
> +			udi_s = &udi[sizeof(UDI_STRIP)-1];
> +
> +	path = g_build_filename (G_DIR_SEPARATOR_S,
> +				 pstore->path, pstore->uuid,
> +				 udi_s, file, NULL);
> +
> +	return path;
> +}
> +
> +/** Init pstore system by reading or generating our uuid.
> + *
> + */
> +void
> +hal_pstore_init (const char *path)
> +{
> +	char *file;
> +	char *uuid;
> +	char hostname[HOST_NAME_MAX];
> +	char *p;
> +	GIOChannel *io;
> +	GError *error = NULL;
> +	int written;
> +
> +	file = g_build_filename (G_DIR_SEPARATOR_S,
> +				 path, "uuid", NULL);
> +	g_file_get_contents (file, &uuid, NULL, NULL);
> +
> +	if (uuid == NULL) {
> +		/* create new uuid and save it to disk */
> +		gethostname (hostname, HOST_NAME_MAX);
> +		uuid = g_strdup_printf ("%s-%lx", hostname, time(NULL));
> +
> +		p = g_build_path (G_DIR_SEPARATOR_S,
> +				  path, NULL);
> +		create_path(p);
> +		g_free(p);
> +
> +		io = g_io_channel_new_file (file, "w", &error);
> +		if (error != NULL) {
> +			HAL_WARNING (("error creating file %s", error->message));
> +			g_error_free (error);
> +			goto exit;
> +		}
> +		g_io_channel_write_chars (io, uuid, -1, &written, &error);
> +		g_io_channel_shutdown (io, TRUE, NULL);
> +		if (error != NULL) {
> +			HAL_WARNING (("error writing to file %s", error->message));
> +			g_error_free (error);
> +			goto exit;
> +		}
> +	}
> +
> +	HAL_DEBUG (("uuid is %s", uuid));
> +	pstore_uuid = uuid;
> +
> +exit:
> +	g_free (file);
> +}
> +
> +
> +/** Open pstore on the given location.
> + *
> + */
> +HalPStore
> +*hal_pstore_open (const char *path)
> +{
> +	HalPStore *pstore;
> +	char *p;
> +
> +	pstore = g_new0 (HalPStore, 1);
> +
> +	pstore->path = g_strdup (path);
> +
> +	pstore->uuid = pstore_uuid;
> +
> +	p = g_build_path (G_DIR_SEPARATOR_S,
> +			  pstore->path, pstore->uuid, NULL);
> +	create_path(p);
> +	g_free(p);
> +
> +	HAL_DEBUG (("opened pstore at %s/%s/", pstore->path, pstore->uuid));
> +
> +	return pstore;
> +}
> +
> +/** Turn around the OPEN sign, in the store's entrance :)
> + *
> + */
> +void
> +hal_pstore_close (HalPStore *pstore)
> +{
> +	g_free (pstore->path);
> +	g_free (pstore->uuid);
> +	g_free (pstore);
> +}

Are you sure you want to free pstore->uuid here?  If you do, pstore_uuid
will be garbage if you try to call hal_pstore_open() without first
calling hal_pstore_init().  You probably want to either g_strdup() it in
hal_store_open() or not free it at all (and declare it const in the
struct definition).

On a related note, you might want to check in hal_pstore_open() that
hal_pstore_init() is called first.  Or just make hal_pstore_init()
static and have hal_pstore_open() lazily call it.

Other than that, it looks pretty good to me.  Hopefully David can find
the time to also look it over and give it the OK.

Thanks,
Joe


_______________________________________________
hal mailing list
hal at freedesktop.org
http://freedesktop.org/mailman/listinfo/hal