PATCH: context based libsmbclient

[Andrew Bartlett]

Richard Sharpe wrote:
> 
> On Thu, 20 Jun 2002, Tom Jansen wrote:
> 
> >
> > > > Attached .... now :-)
> > >
> > > OK, I hope to look at it today ... Thanks ...
> >
> > Be sure to grab the latest version because I added a few fixes:
> > o Makefile quirk (stupid me)
> > o libsmbclient.h fixup so programs including libsmbclient.h can compile
> >
> > http://niihau.student.utwente.nl/~sketch/libsmbc2-1.diff
> 
> OK, I won't get to this until next week now after I come back from Disney

Firstly, this patch looks good, thankyou *very* much for your work on
it.

I do however have some comments:

I don't like the seperation between SMBCCTX * smbc_new_context(void) and
smbc_init_context().  I think that smbc_init_context() should do the
new_context implicitly - it should not be exposed as part of the
interface.

+
+/**@ingroup misc
+ * Delete a SBMCCTX (a context) acquired from smbc_new_context().
+ *
+ * The context will be deleted if possible.
+ *
+ * @return          Returns 0 on succes. Returns 1 on failure with
errno set:
+ *                  - EBUSY Server connections are still used, Files
are open or cache 
+ *                          could not be purged
+ *                  - EBADF context == NULL
+ *
+ * @see             smbc_new_context()
+ *
+ * @note            Do not forget to smbc_init_context() the returned
SMBCCTX pointer !
+ */
+int smbc_free_context(SMBCCTX * context);
+

Firstly, that @note is a cut-n-paste bug.  But while I like this (don't
free if still used) I think we need a 'shutdown' interface, so programs
have somthing to call on their way out (becouse otherwise programs with
just exit() and let the OS clean up - not the best way...).


+
+
+/****************************************************************************
+Send a keepalive packet to the server
+****************************************************************************/
+BOOL cli_send_keepalive(struct cli_state *cli)
+{
+        if (cli->fd == -1) {
+                DEBUG(3, ("cli_send_keepalive: fd == -1\n"));
+                return False;
+        }
+        if (!send_keepalive(cli->fd)) {
+                close(cli->fd);
+                cli->fd = -1;
+                DEBUG(0,("Error sending keepalive packet to
client.\n"));

that should be *server*.

+                return False;
+        }
+        return True;
+}


+       if (!(srvcache = malloc(sizeof(*srvcache)))) {
+               errno = ENOMEM;
+               DEBUG(3, ("Not enough space for server cache
allocation\n"));
+               return 1;
+       }

Are we really allowed to twiddle with errno?  If malloc failed, didn't
the OS fill in the correct error number?  (Which might not be ENOMEM for
some very werid circumstance).

-       pstrcpy(workgroup, lp_workgroup());
+       /* HELP !!! Which workgroup should I use ? Or are they always
the same -- Tom */ 
+       pstrcpy(workgroup, ocontext->workgroup);

This needs work.  The workgroup here actually needs to be specified by
the user - becouse its used to figure out what domain the user belongs
to, and is used by the server in authenticaion.  If it doesn't match the
server's domain or one of its trusted domains, it is *assumed* to be the
server's domain (so that works fine for most cases) but if the user is
in a trusted domain, it breaks.

The 'get username/password' stuff needs to take a 'domain' argument.  We
might also allow that domain to be in the username (as domain\username),
but I don't like these hacks.

+off_t smbc_lseek(int fd, off_t offset, int whence)
+{
+       SMBCFILE * file = (SMBCFILE *) fd;
+       return statcont->lseek(statcont, file, offset, whence);
+}
+

While I like the idea, I think a linear serch of the files in the linked
list
would be better.  sizeof(int) != sizeof(void *) on some platforms.


+int smbc_stat(const char *url, struct stat *st)
+{
+               return statcont->stat(statcont, url, st);
+}
+

This might just be diff/space/tab interaction, but watch that indent.

Again, thanks for the patch, it looks pretty good.

Andrew Bartlett

-- 
Andrew Bartlett                                 abartlet at pcug.org.au
Manager, Authentication Subsystems, Samba Team  abartlet at samba.org
Student Network Administrator, Hawker College   abartlet at hawkerc.net
http://samba.org     http://build.samba.org     http://hawkerc.net