Subject: Re: libssh2_knownhost_*

Re: libssh2_knownhost_*

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Thu, 28 May 2009 16:09:51 +0200 (CEST)

On Thu, 28 May 2009, Alexander Lamaison wrote:

> Thanks for all the great work Daniel. I'm very pleased to see this API
> appear. After a quick skim of the code a few issues came to mind:

Thanks for your feedback!

> libssh2_knownhost_dumpfile: The name of this function implies something
> related to debugging but it isn't. I would suggest something like
> knownhost_persistfile, knownhost_writefile, or knownhost_savefile.
> 'persist' has the advantage of making it appear next to its sibling 'parse'
> when ordered alphabetically ;)

Agreed. But I think I favor "savefile" out of those options.

> filename parameters: The knownhost_*file functions take a const char*
> filename parameter which they proceed to open and use. While this is common
> in APIs, I would argue that it is better to take a FILE* and allow the
> client code to open the file however it sees fit. The FILE interface was
> purposely designed to allow any 'file-like object' to be used as a regular
> disk file. An example of this is the glibc fmemopen function that creates a
> memory-backed file.

Hm right. I've faced troubles with that in the past. In windows for example,
you can's pass such a file handle from an application to a DLL so it'd be
impossible to use there!

Posssibly we should consider the API to instead make full lines and let the
app save those whereever it seems fit. Most people will not be able to get
FILE * pointers to store in memory anywaay. We'd need somsthing similar done
for the read function too then.

It should also be noted that parsefile and savefile both are "convenience"
functions that can in fact be done by the app itself using the other functions
if wanted.

> Whole-file parse/writeback: As I understand it, using the API goes roughly:
> init parsefile (add | del | check | get)* dumpfile
> My worry here is that we will lose information between parsefile and
> dumpfile such as the order of the existing entries and any comment
> strings (iirc, these are allowed after #). In other words:
> file != dumpfile(parsefile(file))
> I've not had time to study the code in detail so I apologise if this
> is already being taken care of.

Right, it will happily ignore comments, blank lines and lines with formats it
doesn't understand on read and savefile will only simply store all known keys.
The code only keeps a collection of keys, it doesn't know about files or
comments from files etc. I would also hesitate to attempt to keep comments
etc, since it'll be hard to get right and what would we do with them if hosts
are removed or added etc?

I think I would instead suggest that if you really want to maintain the
known_host file as much as possible you shouldn't use the savefile option but
instead rather append new keys to the existing file.

-- 
  / daniel.haxx.se
------------------------------------------------------------------------------
Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT 
is a gathering of tech-side developers & brand creativity professionals. Meet
the minds behind Google Creative Lab, Visual Complexity, Processing, & 
iPhoneDevCamp as they present alongside digital heavyweights like Barbarian 
Group, R/GA, & Big Spaceship. http://p.sf.net/sfu/creativitycat-com 
_______________________________________________
libssh2-devel mailing list
libssh2-devel_at_lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libssh2-devel
Received on 2009-05-28