Re: [Bug-wget] SIDB

[Micah Cowan]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Rodrigo S Wanderley wrote:
> Hi,
> 
> I'm trying to implement the Session Info Database.  I'm on the beginning
> of the development, so the code still does not handle with errors.
> Also, the reader and some other functionality are missing and am
> handling only handling the first use case (see wiki).
> 
> Following is a diff against mainline, any comments or suggestions would
> be appreciated.  Am sending the code now so I can see if anyone spots
> any bad design issues that are better fixed at an early stage.

Hi Rod, and thanks!

Comments below.

> +sidb_writer *
> +sidb_writer_start(const char *filename)
> +{
> +  sidb_writer *w = xnew(sidb_writer);
> +  if (w == NULL)
> +    {
> +      goto err;
> +    }
> +
> +  w->fname = xstrdup(filename);
> +  if (w->fname == NULL)
> +    {
> +      goto err;
> +    }
> +
> +  w->we_list = NULL;
> +  return w;
> +
> + err:
> +  xfree_null(w);
> +  return NULL;
> +}

Actually, xnew() and xstrdup() can't ever, ever return NULL: they exit
with an error if they can't allocate memory. IOW, they already do your
error handling for you, so you don't need to do it here (and elsewhere). :)

> +
> +/* write data to disk */
> +void
> +sidb_writer_flush(sidb_writer *w)

Actually, an important feature for SIDB is that it needs to flush on
creation of every entry, so that as much as possible may be recovered in
the event of sudden failure; so there should be no need for this function.

(The easiest way to deal with flushing every entry is probably to ensure
that line-buffering-mode is set on the FILE* via setvbuf).

This also means that:

> +sidb_entry *
> +sidb_entry_new(sidb_writer *w, const char *uri)

should always write something to the file, and so should:

> +void
> +sidb_entry_redirect(sidb_entry *we, const char *uri,
> +                        int redirect_http_status_code)

> +void
> +sidb_entry_local_path(sidb_entry *we,
> +                   const char *fname)


> +void
> +sidb_entry_remove(sidb_entry *we)

Since we're now writing directly to file, you probably don't need this.

> +sidb_entry *
> +sidb_lookup_uri(sidb_reader *sr, const char *uri)
> +{
> +  sidb_entry *e;
> +  sidb_redirect *r;
> +
> +  for (e = sr->we_list; e != NULL; e = e->next)
> +    {
> +      if (are_urls_equal(e->uri, uri))
> +     return e;
> +
> +      /* check redirects */
> +      for (r = e->redirects; r != NULL; r = r->next)
> +     {
> +       if (are_urls_equal(r->uri, uri))
> +         return e;
> +     }
> +    }
> +
> +  return NULL;                       /* uri not found */
> +}

OK for now. We'll definitely want to implement this as a hash lookup, or
a tree at least, before we put it into production.

- --
Micah J. Cowan
Programmer, musician, typesetting enthusiast, gamer.
Maintainer of GNU Wget and GNU Teseq
http://micah.cowan.name/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpc7I0ACgkQ7M8hyUobTrHRUQCfSaHbDMBGK43h6Uc6AbiuPOUT
pvcAn3/6dDFRf74zgXKRLUoQTOZZKgIL
=FlWb
-----END PGP SIGNATURE-----