On Fri, Dec 3, 2010 at 11:09 AM, <ronniesahlb...@gmail.com> wrote:
> @@ -0,0 +1,215 @@
> +/*
> + Copyright (C) 2010 by Ronnie Sahlberg <ronniesahlb...@gmail.com>
> +
> + 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.
You want the library to be GPL, not LGPL?
> +struct iscsi_context *
> +iscsi_create_context(const char *initiator_name)
> +{
> + struct iscsi_context *iscsi;
> +
> + iscsi = malloc(sizeof(struct iscsi_context));
> + if (iscsi == NULL) {
> + return NULL;
> + }
> +
> + bzero(iscsi, sizeof(struct iscsi_context));
> +
> + iscsi->initiator_name = strdup(initiator_name);
> + if (iscsi->initiator_name == NULL) {
> + free(iscsi);
> + return NULL;
> + }
> +
> + iscsi->fd = -1;
> +
> + /* use a "random" isid */
> + srandom(getpid() ^ time(NULL));
The random number generator has global state and the library may
interfere if the program also uses the srandom() interface.
> + iscsi_set_random_isid(iscsi);
> +
> + return iscsi;
> +}
> +
> +int
> +iscsi_set_random_isid(struct iscsi_context *iscsi)
> +{
> + iscsi->isid[0] = 0x80;
> + iscsi->isid[1] = random()&0xff;
> + iscsi->isid[2] = random()&0xff;
> + iscsi->isid[3] = random()&0xff;
> + iscsi->isid[4] = 0;
> + iscsi->isid[5] = 0;
> +
> + return 0;
> +}
Is there an interface to set a specific isid value? Users will
probably want to use a permanent value.
> +int
> +iscsi_destroy_context(struct iscsi_context *iscsi)
> +{
> + struct iscsi_pdu *pdu;
> +
> + if (iscsi == NULL) {
> + return 0;
> + }
> + if (iscsi->initiator_name != NULL) {
> + free(discard_const(iscsi->initiator_name));
> + iscsi->initiator_name = NULL;
> + }
> + if (iscsi->target_name != NULL) {
> + free(discard_const(iscsi->target_name));
> + iscsi->target_name = NULL;
> + }
> + if (iscsi->alias != NULL) {
> + free(discard_const(iscsi->alias));
> + iscsi->alias = NULL;
> + }
All these if statements are unnecessary. free(NULL) is a nop.
> + if (iscsi->fd != -1) {
> + iscsi_disconnect(iscsi);
Perhaps disconnect before freeing struct members?
> + }
> +
> + if (iscsi->inbuf != NULL) {
> + free(iscsi->inbuf);
> + iscsi->inbuf = NULL;
> + iscsi->insize = 0;
> + iscsi->inpos = 0;
> + }
> +
> + while ((pdu = iscsi->outqueue)) {
> + SLIST_REMOVE(&iscsi->outqueue, pdu);
> + pdu->callback(iscsi, SCSI_STATUS_CANCELLED, NULL,
> + pdu->private_data);
> + iscsi_free_pdu(iscsi, pdu);
> + }
> + while ((pdu = iscsi->waitpdu)) {
> + SLIST_REMOVE(&iscsi->waitpdu, pdu);
> + pdu->callback(iscsi, SCSI_STATUS_CANCELLED, NULL,
> + pdu->private_data);
> + iscsi_free_pdu(iscsi, pdu);
> + }
Could these callbacks expect iscsi to still be valid? A safer order
would seem to be: disconnect, cancel all, free.
> +
> + if (iscsi->error_string != NULL) {
> + free(iscsi->error_string);
> + }
> +
> + free(iscsi);
> +
> + return 0;
> +}
> +
> +
> +
> +void
> +iscsi_set_error(struct iscsi_context *iscsi, const char *error_string, ...)
> +{
> + va_list ap;
> + char *str;
> +
> + va_start(ap, error_string);
> + if (vasprintf(&str, error_string, ap) < 0) {
This function is available in GNU and BSD. Not sure what level of
portability you are targeting (e.g. what about Windows)?
> + /* not much we can do here */
You need to assign str = NULL here. Otherwise its value is undefined
on GNU systems.
> + }
> + if (iscsi->error_string != NULL) {
> + free(iscsi->error_string);
> + }
> + iscsi->error_string = str;
> + va_end(ap);
> +}
> +
> +
> +char *
const char *?
> +iscsi_get_error(struct iscsi_context *iscsi)
> +{
> + return iscsi->error_string;
> +}
Stefan
