From: Andre Noll Date: Tue, 25 Apr 2017 16:04:57 +0000 (+0200) Subject: Merge branch 'maint' X-Git-Tag: v0.6.0~5 X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=869fa1d76e7f88470120552792ca71068b49a747;hp=-c Merge branch 'maint' 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(). --- 869fa1d76e7f88470120552792ca71068b49a747 diff --combined aft.c index 7d3a6e0d,1afc16bc..22890e56 --- a/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; @@@ -409,37 -408,42 +409,37 @@@ 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; @@@ -1095,7 -1072,7 +1095,7 @@@ 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; /* @@@ -1108,8 -1085,7 +1108,8 @@@ 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 254bcb8d,32b6895d..51795847 --- a/audiod.c +++ 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) @@@ -318,11 -332,11 +319,11 @@@ } /* * 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) { @@@ -339,7 -353,7 +340,7 @@@ 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); @@@ -1059,10 -1073,12 +1060,10 @@@ 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; @@@ -1103,15 -1123,19 +1104,15 @@@ 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 023d78d0,eaa1cfb8..6c23fdd9 --- a/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; @@@ -629,7 -629,7 +629,7 @@@ */ 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; } @@@ -856,22 -844,28 +856,22 @@@ * 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); @@@ -886,9 -880,7 +886,9 @@@ 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 6bf2d641,b4935f0d..7394bf91 --- a/wma_afh.c +++ 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); @@@ -340,6 -342,8 +342,8 @@@ 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); @@@ -351,8 -355,6 +355,6 @@@ *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;