]> git.tuebingen.mpg.de Git - paraslash.git/commitdiff
Merge branch 'maint'
authorAndre Noll <maan@tuebingen.mpg.de>
Tue, 25 Apr 2017 16:04:57 +0000 (18:04 +0200)
committerAndre Noll <maan@tuebingen.mpg.de>
Tue, 25 Apr 2017 16:04:57 +0000 (18:04 +0200)
A couple of fixes found by the clang static analyzer.

* maint:
  aft: Check return value of load_afsi().
  audiod: Avoid reading garbage in get_time_string().
  net: Always initialize struct sockaddr_storage.
  wma_afh: Fix two bugs in convert_utf8_to_utf16().

1  2 
aft.c
audiod.c
net.c
wma_afh.c

diff --combined aft.c
index 7d3a6e0d98a01e40974f22285e9be2d9fac01b46,1afc16bc542b3812687af3a58df97b941586d885..22890e569c0fdc73561644d1a346f7792853b38e
--- 1/aft.c
--- 2/aft.c
+++ b/aft.c
@@@ -335,8 -335,8 +335,8 @@@ enum afhi_offsets 
        CHUNKS_TOTAL_OFFSET = 20,
        /** The length of the audio file header (4 bytes). */
        HEADER_LEN_OFFSET = 24,
 -      /** Was: The start of the audio file header (4 bytes). */
 -      AFHI_UNUSED2_OFFSET = 28,
 +      /** Size of the largest chunk in bytes. (4 bytes). */
 +      AFHI_MAX_CHUNK_SIZE_OFFSET = 28,
        /** The seconds part of the chunk time (4 bytes). */
        CHUNK_TV_TV_SEC_OFFSET = 32,
        /** The microseconds part of the chunk time (4 bytes). */
@@@ -376,7 -376,7 +376,7 @@@ static void save_afhi(struct afh_info *
        write_u8(buf + AFHI_CHANNELS_OFFSET, afhi->channels);
        write_u32(buf + CHUNKS_TOTAL_OFFSET, afhi->chunks_total);
        write_u32(buf + HEADER_LEN_OFFSET, afhi->header_len);
 -      write_u32(buf + AFHI_UNUSED2_OFFSET, 0);
 +      write_u32(buf + AFHI_MAX_CHUNK_SIZE_OFFSET, afhi->max_chunk_size);
        write_u32(buf + CHUNK_TV_TV_SEC_OFFSET, afhi->chunk_tv.tv_sec);
        write_u32(buf + CHUNK_TV_TV_USEC_OFFSET, afhi->chunk_tv.tv_usec);
        p = buf + AFHI_INFO_STRING_OFFSET;
@@@ -398,7 -398,6 +398,7 @@@ static void load_afhi(const char *buf, 
        afhi->channels = read_u8(buf + AFHI_CHANNELS_OFFSET);
        afhi->chunks_total = read_u32(buf + CHUNKS_TOTAL_OFFSET);
        afhi->header_len = read_u32(buf + HEADER_LEN_OFFSET);
 +      afhi->max_chunk_size = read_u32(buf + AFHI_MAX_CHUNK_SIZE_OFFSET);
        afhi->chunk_tv.tv_sec = read_u32(buf + CHUNK_TV_TV_SEC_OFFSET);
        afhi->chunk_tv.tv_usec = read_u32(buf + CHUNK_TV_TV_USEC_OFFSET);
        afhi->techinfo = (char *)buf + AFHI_INFO_STRING_OFFSET;
        afhi->tags.comment = afhi->tags.album + strlen(afhi->tags.album) + 1;
  }
  
 +/* Only used for saving the chunk table, but not for loading. */
  static unsigned sizeof_chunk_table(struct afh_info *afhi)
  {
 -      if (!afhi)
 +      if (!afhi || !afhi->chunk_table)
                return 0;
        return 4 * (afhi->chunks_total + 1);
  }
  
 -static uint32_t save_chunk_table(struct afh_info *afhi, char *buf)
 +static void save_chunk_table(struct afh_info *afhi, char *buf)
  {
 -      int i;
 -      uint32_t max = 0, old = 0;
 +      uint32_t n;
  
 -      for (i = 0; i <= afhi->chunks_total; i++) {
 -              uint32_t val = afhi->chunk_table[i];
 -              write_u32(buf + 4 * i, val);
 -              /*
 -               * If the first chunk is the header, do not consider it for the
 -               * calculation of the largest chunk size.
 -               */
 -              if (i == 0 || (i == 1 && afhi->header_len > 0)) {
 -                      old = val;
 -                      continue;
 -              }
 -              max = PARA_MAX(max, val - old);
 -              old = val;
 -      }
 -      return max;
 +      if (!afhi->chunk_table)
 +              return;
 +      for (n = 0; n <= afhi->chunks_total; n++)
 +              write_u32(buf + 4 * n, afhi->chunk_table[n]);
  }
  
 -static void load_chunk_table(struct afh_info *afhi, char *buf)
 +static void load_chunk_table(struct afh_info *afhi, const struct osl_object *ct)
  {
        int i;
 +      size_t sz;
  
 -      afhi->chunk_table = para_malloc(sizeof_chunk_table(afhi));
 -      for (i = 0; i <= afhi->chunks_total; i++)
 -              afhi->chunk_table[i] = read_u32(buf + 4 * i);
 +      if (!ct->data || ct->size < 4) {
 +              afhi->chunk_table = NULL;
 +              return;
 +      }
 +      sz  = PARA_MIN(((size_t)afhi->chunks_total + 1) * 4, ct->size) + 1;
 +      afhi->chunk_table = para_malloc(sz);
 +      for (i = 0; i <= afhi->chunks_total && i * 4 + 3 < ct->size; i++)
 +              afhi->chunk_table[i] = read_u32(ct->data + 4 * i);
  }
  
  /**
@@@ -634,13 -638,7 +634,13 @@@ static int save_afd(struct audio_file_d
                goto err;
        buf = shm_afd;
        buf += sizeof(*afd);
 -      afd->max_chunk_size = save_chunk_table(&afd->afhi, buf);
 +      save_chunk_table(&afd->afhi, buf);
 +      if (afd->afhi.max_chunk_size == 0) { /* v0.5.x on-disk afhi */
 +              set_max_chunk_size(&afd->afhi);
 +              PARA_NOTICE_LOG("max chunk size unset, re-add required\n");
 +      } else
 +              PARA_INFO_LOG("using max chunk size from afhi\n");
 +      afd->max_chunk_size = afd->afhi.max_chunk_size;
        *(struct audio_file_data *)shm_afd = *afd;
        shm_detach(shm_afd);
        return shmid;
@@@ -665,22 -663,14 +665,22 @@@ int load_afd(int shmid, struct audio_fi
  {
        void *shm_afd;
        int ret;
 +      struct osl_object obj;
  
        ret = shm_attach(shmid, ATTACH_RO, &shm_afd);
        if (ret < 0)
                return ret;
 +      ret = shm_size(shmid, &obj.size);
 +      if (ret < 0)
 +              goto detach;
        *afd = *(struct audio_file_data *)shm_afd;
 -      load_chunk_table(&afd->afhi, shm_afd + sizeof(*afd));
 +      obj.data = shm_afd + sizeof(*afd);
 +      obj.size -= sizeof(*afd);
 +      load_chunk_table(&afd->afhi, &obj);
 +      ret = 1;
 +detach:
        shm_detach(shm_afd);
 -      return 1;
 +      return ret;
  }
  
  static int get_local_time(uint64_t *seconds, char *buf, size_t size,
@@@ -830,11 -820,7 +830,11 @@@ static int print_chunk_table(struct ls_
                (long unsigned) d->afhi.chunk_tv.tv_usec
        );
        buf = chunk_table_obj.data;
 -      for (i = 0; i <= d->afhi.chunks_total; i++)
 +      for (
 +              i = 0;
 +              i <= d->afhi.chunks_total && 4 * i + 3 < chunk_table_obj.size;
 +              i++
 +      )
                para_printf(b, "%u ", (unsigned) read_u32(buf + 4 * i));
        para_printf(b, "\n");
        ret = 1;
@@@ -949,8 -935,6 +949,8 @@@ static int print_list_item(struct ls_da
        WRITE_STATUS_ITEM(b, SI_CHUNK_TIME, "%lu\n", tv2ms(&afhi->chunk_tv));
        WRITE_STATUS_ITEM(b, SI_NUM_CHUNKS, "%" PRIu32 "\n",
                afhi->chunks_total);
 +      WRITE_STATUS_ITEM(b, SI_MAX_CHUNK_SIZE, "%" PRIu32 "\n",
 +              afhi->max_chunk_size);
        WRITE_STATUS_ITEM(b, SI_TECHINFO, "%s\n", afhi->techinfo);
        WRITE_STATUS_ITEM(b, SI_ARTIST, "%s\n", afhi->tags.artist);
        WRITE_STATUS_ITEM(b, SI_TITLE, "%s\n", afhi->tags.title);
@@@ -1070,15 -1054,8 +1070,15 @@@ again
        d->afhi.chunk_table = afd->afhi.chunk_table = NULL;
        ret = osl(osl_open_disk_object(audio_file_table, current_aft_row,
                AFTCOL_CHUNKS, &chunk_table_obj));
 -      if (ret < 0)
 -              return ret;
 +      if (ret < 0) {
 +              if (!afh_supports_dynamic_chunks(d->afsi.audio_format_id))
 +                      return ret;
 +              PARA_INFO_LOG("no chunk table for %s\n", d->path);
 +              chunk_table_obj.data = NULL;
 +              chunk_table_obj.size = 0;
 +      } else {
 +              PARA_INFO_LOG("chunk table: %zu bytes\n", chunk_table_obj.size);
 +      }
        ret = mmap_full_file(d->path, O_RDONLY, &map.data, &map.size, &afd->fd);
        if (ret < 0)
                goto out;
        save_afsi(&new_afsi, &afsi_obj); /* in-place update */
  
        afd->audio_format_id = d->afsi.audio_format_id;
 -      load_chunk_table(&afd->afhi, chunk_table_obj.data);
 +      load_chunk_table(&afd->afhi, &chunk_table_obj);
        aced.aft_row = current_aft_row;
        aced.old_afsi = &d->afsi;
        /*
        ret = save_afd(afd);
  out:
        free(afd->afhi.chunk_table);
 -      osl_close_disk_object(&chunk_table_obj);
 +      if (chunk_table_obj.data)
 +              osl_close_disk_object(&chunk_table_obj);
        if (ret < 0) {
                PARA_ERROR_LOG("%s: %s\n", d->path, para_strerror(-ret));
                ret = score_delete(current_aft_row);
@@@ -1760,7 -1736,6 +1760,7 @@@ static int com_add_callback(struct afs_
                        &objs[AFTCOL_AFHI]));
                if (ret < 0)
                        goto out;
 +              /* truncate the file to size zero if there is no chunk table */
                ret = osl(osl_update_object(audio_file_table, row, AFTCOL_CHUNKS,
                        &objs[AFTCOL_CHUNKS]));
                if (ret < 0)
@@@ -2297,7 -2272,9 +2297,9 @@@ static int copy_selector_info(__a_unuse
        ret = get_afsi_object_of_row(row, &target_afsi_obj);
        if (ret < 0)
                return ret;
-       load_afsi(&target_afsi, &target_afsi_obj);
+       ret = load_afsi(&target_afsi, &target_afsi_obj);
+       if (ret < 0)
+               return ret;
        old_afsi = target_afsi;
        if (cad->flags & CPSI_FLAG_COPY_LYRICS_ID)
                target_afsi.lyrics_id = cad->source_afsi.lyrics_id;
diff --combined audiod.c
index 254bcb8d8c4bac2947e52c3310fb7051fe5f1153,32b6895dcf0da9393538e649f7241878b89e96c0..517958474f39077ebbbf3b6557d4bab6a6ebf82c
+++ b/audiod.c
@@@ -183,9 -183,22 +183,9 @@@ static uid_t *uid_whitelist
   */
  static struct status_task *stat_task = &status_task_struct;
  
 -/*
 - * The task for handling audiod commands.
 - *
 - * We need two listening sockets for backward compability: on Linux systems
 - * fd[0] is an abstract socket (more precisely, a socket bound to an address in
 - * the abstract namespace), and fd[1] is the usual pathname socket. On other
 - * systems, fd[0] is negative, and only the pathname socket is used.
 - *
 - * For 0.5.x we accept connections on both sockets to make sure that old
 - * para_audioc versions can still connect. New versions use only the abstract
 - * socket. Hence after v0.6.0 we can go back to a single socket, either an
 - * abstract one (Linux) or a pathname socket (all other systems).
 - */
  struct command_task {
 -      /** The local listening sockets. */
 -      int fd[2];
 +      /** The local listening socket. */
 +      int fd;
        /** the associated task structure */
        struct task *task;
  };
@@@ -305,6 -318,7 +305,7 @@@ char *get_time_string(void
                rskip; /* receiver start - sss */
        int slot_num = get_play_time_slot_num();
        struct slot_info *s = slot_num < 0? NULL : &slot[slot_num];
+       bool writer_active = s && s->wns && s->wns[0].btrn;
        char *msg;
  
        if (audiod_status == AUDIOD_OFF)
        }
        /*
         * Valid status items and playing, set length and tmp to the stream
-        * start. We use the slot info and fall back to the info from current
-        * status items if no slot info is available.
+        * start. We use the writer start time from the slot info and fall back
+        * to the info from current status items if no writer is active yet.
         */
        tmp = &stat_task->server_stream_start;
-       if (s && s->wns && s->wns[0].btrn) { /* writer active in this slot */
+       if (writer_active) {
                btr_get_node_start(s->wns[0].btrn, &wstime);
                if (wstime.tv_sec != 0) { /* writer wrote something */
                        if (s->server_stream_start.tv_sec == 0) {
                tv_diff(tmp, &stat_task->sa_time_diff, &sss);
        else
                tv_add(tmp, &stat_task->sa_time_diff, &sss);
-       if (!s || !s->wns || !s->wns[0].btrn || wstime.tv_sec == 0) {
+       if (!writer_active) {
                struct timeval diff;
                tv_diff(now, &sss, &diff);
                seconds = diff.tv_sec + stat_task->offset_seconds;
@@@ -1046,7 -1060,7 +1047,7 @@@ static int parse_stream_args(void
  }
  
  /* does not unlink socket on errors */
 -static void init_local_sockets(struct command_task *ct)
 +static void init_local_socket(struct command_task *ct)
  {
        if (conf.socket_given)
                socket_name = para_strdup(conf.socket_arg);
        PARA_NOTICE_LOG("local socket: %s\n", socket_name);
        if (conf.force_given)
                unlink(socket_name);
 -      ct->fd[0] = create_local_socket(socket_name, 0);
 -      ct->fd[1] = create_local_socket(socket_name,
 -              S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
 -      if (ct->fd[0] >= 0 || ct->fd[1] >= 0)
 +      ct->fd = create_local_socket(socket_name);
 +      if (ct->fd >= 0)
                return;
 -      PARA_EMERG_LOG("%s\n", para_strerror(-ct->fd[1]));
 +      PARA_EMERG_LOG("%s\n", para_strerror(-ct->fd));
        exit(EXIT_FAILURE);
  }
  
@@@ -1089,12 -1105,16 +1090,12 @@@ static int signal_post_select(struct sc
  static void command_pre_select(struct sched *s, void *context)
  {
        struct command_task *ct = context;
 -      int i;
 -
 -      for (i = 0; i < 2; i++)
 -              if (ct->fd[i] >= 0)
 -                      para_fd_set(ct->fd[i], &s->rfds, &s->max_fileno);
 +      para_fd_set(ct->fd, &s->rfds, &s->max_fileno);
  }
  
  static int command_post_select(struct sched *s, void *context)
  {
 -      int ret, i;
 +      int ret;
        struct command_task *ct = context;
        static struct timeval last_status_dump;
        struct timeval tmp, delay;
        ret = task_get_notification(ct->task);
        if (ret < 0)
                return ret;
 -      for (i = 0; i < 2; i++) {
 -              if (ct->fd[i] < 0)
 -                      continue;
 -              ret = handle_connect(ct->fd[i], &s->rfds);
 -              if (ret < 0) {
 -                      PARA_ERROR_LOG("%s\n", para_strerror(-ret));
 -                      if (ret == -E_AUDIOD_TERM) {
 -                              task_notify_all(s, -ret);
 -                              return ret;
 -                      }
 -              } else if (ret > 0)
 -                      force = true;
 -      }
 +      ret = handle_connect(ct->fd, &s->rfds);
 +      if (ret < 0) {
 +              PARA_ERROR_LOG("%s\n", para_strerror(-ret));
 +              if (ret == -E_AUDIOD_TERM) {
 +                      task_notify_all(s, -ret);
 +                      return ret;
 +              }
 +      } else if (ret > 0)
 +              force = true;
        if (force == true)
                goto dump;
  
@@@ -1139,7 -1163,7 +1140,7 @@@ dump
  
  static void init_command_task(struct command_task *ct)
  {
 -      init_local_sockets(ct); /* doesn't return on errors */
 +      init_local_socket(ct); /* doesn't return on errors */
  
        ct->task = task_register(&(struct task_info) {
                .name = "command",
diff --combined net.c
index 023d78d08175f2f3810af6dd387f5eec14a06dcc,eaa1cfb81146b3dc4fd7bb11cd41df447dc0e825..6c23fdd9ce6b248e562e84c23d3e089e4484b178
--- 1/net.c
--- 2/net.c
+++ b/net.c
@@@ -603,7 -603,7 +603,7 @@@ static inline int estimated_header_over
   */
  int generic_max_transport_msg_size(int sockfd)
  {
-       struct sockaddr_storage ss;
+       struct sockaddr_storage ss = {0};
        socklen_t sslen = sizeof(ss);
        int af_type = AF_INET;
  
   */
  char *remote_name(int fd)
  {
-       struct sockaddr_storage ss;
+       struct sockaddr_storage ss = {0};
        const struct sockaddr *sa;
        socklen_t sslen = sizeof(ss);
        char hbuf[NI_MAXHOST], sbuf[NI_MAXSERV];
@@@ -818,37 -818,25 +818,37 @@@ int dccp_available_ccids(uint8_t **ccid
        return nccids;
  }
  
 -/**
 - * Prepare a structure for \p AF_UNIX socket addresses.
 - *
 - * \param u Pointer to the struct to be prepared.
 - * \param name The socket pathname.
 +/*
 + * Prepare a structure for AF_UNIX socket addresses.
   *
 - * This just copies \a name to the sun_path component of \a u.
 + * This just copies name to the sun_path component of u, prepending a zero byte
 + * if abstract sockets are supported.
   *
 - * \return Positive on success, \p -E_NAME_TOO_LONG if \a name is longer
 - * than \p UNIX_PATH_MAX.
 + * The first call to this function tries to bind a socket to the abstract name
 + * space. The result of this test is stored in a static variable. Subsequent
 + * calls read this variable and create abstract sockets on systems that support
 + * them.
   */
 -static int init_unix_addr(struct sockaddr_un *u, const char *name,
 -              bool abstract)
 +static int init_unix_addr(struct sockaddr_un *u, const char *name)
  {
 -      if (strlen(name) + abstract >= UNIX_PATH_MAX)
 +      static int use_abstract;
 +
 +      if (strlen(name) + 1 >= UNIX_PATH_MAX)
                return -E_NAME_TOO_LONG;
        memset(u->sun_path, 0, UNIX_PATH_MAX);
        u->sun_family = PF_UNIX;
 -      strcpy(u->sun_path + abstract, name);
 +      if (use_abstract == 0) { /* executed only once */
 +              int fd = socket(PF_UNIX, SOCK_STREAM, 0);
 +              memcpy(u->sun_path, "\0x\0", 3);
 +              if (bind(fd, (struct sockaddr *)u, sizeof(*u)) == 0)
 +                      use_abstract = 1; /* yes */
 +              else
 +                      use_abstract = -1; /* no */
 +              close(fd);
 +              PARA_NOTICE_LOG("%susing abstract socket namespace\n",
 +                      use_abstract == 1? "" : "not ");
 +      }
 +      strcpy(u->sun_path + (use_abstract == 1? 1 : 0), name);
        return 1;
  }
  
   * Create a socket for local communication and listen on it.
   *
   * \param name The socket pathname.
 - * \param mode The desired permissions of the socket.
   *
   * This function creates a passive local socket for sequenced, reliable,
   * two-way, connection-based byte streams. The socket file descriptor is set to
   * nonblocking mode and listen(2) is called to prepare the socket for
   * accepting incoming connection requests.
   *
 - * If mode is zero, an abstract socket (a non-portable Linux extension) is
 - * created. In this case the socket name has no connection with filesystem
 - * pathnames.
 - *
   * \return The file descriptor on success, negative error code on failure.
   *
   * \sa socket(2), \sa bind(2), \sa chmod(2), listen(2), unix(7).
   */
 -int create_local_socket(const char *name, mode_t mode)
 +int create_local_socket(const char *name)
  {
        struct sockaddr_un unix_addr;
        int fd, ret;
 -      bool abstract = mode == 0;
  
 -      ret = init_unix_addr(&unix_addr, name, abstract);
 +      ret = init_unix_addr(&unix_addr, name);
        if (ret < 0)
                return ret;
        ret = socket(PF_UNIX, SOCK_STREAM, 0);
                ret = -ERRNO_TO_PARA_ERROR(errno);
                goto err;
        }
 -      if (!abstract) {
 +      if (unix_addr.sun_path[0] != 0) { /* pathname socket */
 +              mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP
 +                      | S_IROTH | S_IWOTH;
                ret = -E_CHMOD;
                if (chmod(name, mode) < 0)
                        goto err;
@@@ -925,7 -917,14 +925,7 @@@ int connect_local_socket(const char *na
        fd = socket(PF_UNIX, SOCK_STREAM, 0);
        if (fd < 0)
                return -ERRNO_TO_PARA_ERROR(errno);
 -      /* first try (linux-only) abstract socket */
 -      ret = init_unix_addr(&unix_addr, name, true);
 -      if (ret < 0)
 -              goto err;
 -      if (connect(fd, (struct sockaddr *)&unix_addr, sizeof(unix_addr)) != -1)
 -              return fd;
 -      /* next try pathname socket */
 -      ret = init_unix_addr(&unix_addr, name, false);
 +      ret = init_unix_addr(&unix_addr, name);
        if (ret < 0)
                goto err;
        if (connect(fd, (struct sockaddr *)&unix_addr, sizeof(unix_addr)) != -1)
diff --combined wma_afh.c
index 6bf2d64134539f07237fb31db9a83a7629db3f83,b4935f0d44f30fc6091f8431b99a63f23674aed6..7394bf91768276b92884e1548f61ed791946540d
+++ b/wma_afh.c
@@@ -38,7 -38,8 +38,7 @@@ static int count_frames(const char *buf
                sfc++;
        }
        PARA_INFO_LOG("%d frames, %d superframes\n", fc, sfc);
 -      if (num_superframes)
 -              *num_superframes = sfc;
 +      *num_superframes = sfc;
        return fc;
  }
  
@@@ -67,7 -68,7 +67,7 @@@ static int put_utf8(uint32_t val, char 
                *out++ = in;
                return 1;
        }
 -      bytes = (wma_log2(in) + 4) / 5;
 +      bytes = DIV_ROUND_UP(wma_log2(in), 5);
        shift = (bytes - 1) * 6;
        *out++ = (256 - (256 >> bytes)) | (in >> shift);
        while (shift >= 6) {
@@@ -228,7 -229,6 +228,7 @@@ static int wma_make_chunk_table(char *b
                }
        }
        afhi->chunks_total = j;
 +      set_max_chunk_size(afhi);
        set_chunk_tv(frames_per_chunk, afhi->frequency, &afhi->chunk_tv);
        return 1;
  fail:
@@@ -316,22 -316,24 +316,24 @@@ static const char top_level_header_obje
  
  static int convert_utf8_to_utf16(char *src, char **dst)
  {
-       /*
-        * Without specifying LE (little endian), iconv includes a byte order
-        * mark (e.g. 0xFFFE) at the beginning.
-        */
-       iconv_t cd = iconv_open("UTF-16LE", "UTF-8");
+       iconv_t cd;
        size_t sz, inbytes, outbytes, inbytesleft, outbytesleft;
        char *inbuf, *outbuf;
        int ret;
  
        if (!src || !*src) {
                *dst = para_calloc(2);
-               ret = 0;
-               goto out;
+               return 0;
        }
-       if (cd == (iconv_t) -1)
+       /*
+        * Without specifying LE (little endian), iconv includes a byte order
+        * mark (e.g. 0xFFFE) at the beginning.
+        */
+       cd = iconv_open("UTF-16LE", "UTF-8");
+       if (cd == (iconv_t)-1) {
+               *dst = NULL;
                return -ERRNO_TO_PARA_ERROR(errno);
+       }
        inbuf = src;
        /* even though src is in UTF-8, strlen() should DTRT */
        inbytes = inbytesleft = strlen(src);
        sz = iconv(cd, ICONV_CAST &inbuf, &inbytesleft, &outbuf, &outbytesleft);
        if (sz == (size_t)-1) {
                ret = -ERRNO_TO_PARA_ERROR(errno);
+               free(*dst);
+               *dst = NULL;
                goto out;
        }
        assert(outbytes >= outbytesleft);
        *dst = outbuf;
        PARA_INFO_LOG("converted %s to %d UTF-16 bytes\n", src, ret);
  out:
-       if (ret < 0)
-               free(*dst);
        if (iconv_close(cd) < 0)
                PARA_WARNING_LOG("iconv_close: %s\n", strerror(errno));
        return ret;