Skip to content

Commit

Permalink
build, general: Reduce type and header checking, assume more C99
Browse files Browse the repository at this point in the history
This introduces a single compile check that ensures that a number of
assumed C99 features are there. A number of HAVE_FOO_H and SIZEOF_FOO
macros are gone now. We just use standard C99 headers.

A similar grouped test should follow for POSIX stuff. A bit less
cruft in configure logic and preprocessing. Lots of SIZE_P and
similar are gone, also fixing a number of occasions where
the debug printf format didn't match the suppplied type (cast
missing).



git-svn-id: svn://scm.orgis.org/mpg123/trunk@5377 35dc7657-300d-0410-a2e5-dc2837fedb53
  • Loading branch information
thor committed Oct 5, 2023
1 parent 8a2d406 commit 72157ad
Show file tree
Hide file tree
Showing 36 changed files with 180 additions and 198 deletions.
1 change: 1 addition & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ endif

# mpg123.spec is autogenerated but needs to be present in tarball!
EXTRA_DIST += \
test_c99.c \
mpg123.spec \
makedll.sh \
windows-builds.sh \
Expand Down
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
- libmpg123, libsyn123: always ifdef LFS_LARGEFILE_64 (not just if)
- libsyn123: re-introduce _32 wrappers in addition to suffix-less ones
(regression from 1.31, bug 363)
- build:
-- Group C99 feature checks and make several standard headers
mandatory.
-- Get rid of SIZE_P, OFF_P and friends.

1.32.2
------
Expand Down
34 changes: 11 additions & 23 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -1183,30 +1183,18 @@ AC_SUBST(YASMFLAGS)

dnl ############## Really basic headers, needed for other checks.

AC_MSG_CHECKING([for basic C99 features])
AC_LINK_IFELSE(
[AC_LANG_SOURCE([`cat "$srcdir/test_c99.c"`])],
[have_c99=yes; AC_MSG_RESULT([yes])],
[have_c99=no; AC_MSG_RESULT([no])]
)
if test "x$have_c99" = xno; then
AC_MSG_ERROR([Your toolchain lacks basic C99 features. See test_c99.c for the requirements.])
fi

dnl Is it too paranoid to specifically check for stdint.h and limits.h?
AC_CHECK_HEADERS([stdio.h stdlib.h string.h unistd.h sched.h sys/ioctl.h sys/types.h stdint.h limits.h inttypes.h sys/time.h sys/wait.h sys/resource.h sys/signal.h signal.h sys/select.h dirent.h sys/stat.h])

dnl ############## Types

# Using the lower level macros instead of AC_TYPE_* for compatibility with not freshest autoconf.
# Re-think this list. Some things just should be a given. We assume C99 now.
# Rather think about assumptions like int64_t >= ptrdiff_t.
AC_CHECK_TYPE(size_t, unsigned long)
AC_CHECK_TYPE(uintptr_t, unsigned long)
AC_CHECK_TYPE(ssize_t, long)
AC_CHECK_TYPE(ptrdiff_t, long)
AC_CHECK_TYPE(int32_t, int)
AC_CHECK_TYPE(int64_t, long long)
AC_CHECK_TYPE(uint32_t, unsigned int)
AC_CHECK_TYPE(int16_t, short)
AC_CHECK_TYPE(uint16_t, unsigned short)
AC_CHECK_SIZEOF(size_t)
AC_CHECK_SIZEOF(ssize_t)
AC_CHECK_SIZEOF(int32_t)
AC_CHECK_SIZEOF(long)

# We do not need to know if off_t actually changes size using
# _FILE_OFFSET_BITS, only need to know what size it has.
AC_CHECK_HEADERS([unistd.h sched.h sys/ioctl.h sys/types.h sys/time.h sys/wait.h sys/resource.h sys/signal.h signal.h sys/select.h dirent.h sys/stat.h])

dnl ############## LFS stuff

Expand Down
9 changes: 0 additions & 9 deletions ports/cmake/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,12 @@ option(PORTABLE_API "Only build portable library API (no off_t, no internal I/O.
check_include_file("arpa/inet.h" HAVE_ARPA_INET_H)
check_include_file("dirent.h" HAVE_DIRENT_H)
check_include_file("dlfcn.h" HAVE_DLFCN_H)
check_include_file("inttypes.h" HAVE_INTTYPES_H)
check_include_file("langinfo.h" HAVE_LANGINFO_H)
check_include_file("limits.h" HAVE_LIMITS_H)
check_include_file("locale.h" HAVE_LOCALE_H)
check_include_file("netdb.h" HAVE_NETDB_H)
check_include_file("netinet/in.h" HAVE_NETINET_IN_H)
check_include_file("sched.h" HAVE_SCHED_H)
check_include_file("signal.h" HAVE_SIGNAL_H)
check_include_file("stdio.h" HAVE_STDIO_H)
check_include_file("stdint.h" HAVE_STDINT_H)
check_include_file("stdlib.h" HAVE_STDLIB_H)
check_include_file("string.h" HAVE_STRING_H)
check_include_file("strings.h" HAVE_STRINGS_H)
check_include_file("sys/ioctl.h" HAVE_SYS_IOCTL_H)
check_include_file("sys/ipc.h" HAVE_SYS_IPC_H)
Expand Down Expand Up @@ -70,7 +64,6 @@ check_function_exists(execvp HAVE_EXECVP)
check_function_exists(ctermid HAVE_CTERMID)
check_function_exists(clock_gettime HAVE_CLOCK_GETTIME)

check_type_size(long SIZEOF_LONG)
check_type_size(off_t SIZEOF_OFF_T)

if(SIZEOF_OFF_T LESS 8)
Expand Down Expand Up @@ -135,8 +128,6 @@ if(NOT LFS_INSENSITIVE)
int main() { return 0; }"
LFS_SENSITIVE)
endif()
check_type_size(long SIZEOF_LONG)
check_type_size(off_t SIZEOF_OFF_T)

if(WIN32 AND HAVE_WINDOWS_H)
check_function_exists(GetThreadErrorMode HAVE_GETTHREADERRORMODE)
Expand Down
7 changes: 0 additions & 7 deletions ports/cmake/src/config.cmake.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#cmakedefine HAVE_DLFCN_H 1
#cmakedefine HAVE_INTTYPES_H 1
#cmakedefine HAVE_LANGINFO_H 1
#cmakedefine HAVE_LIMITS_H 1
#cmakedefine HAVE_LOCALE_H 1
#cmakedefine HAVE_NL_LANGINFO 1
#cmakedefine HAVE_RANDOM 1
Expand All @@ -42,16 +41,12 @@
#cmakedefine HAVE_USELOCALE 1
#cmakedefine HAVE_SETPRIORITY 1
#cmakedefine HAVE_SIGNAL_H 1
#cmakedefine HAVE_STDIO_H 1
#cmakedefine HAVE_STDINT_H 1
#cmakedefine HAVE_STDLIB_H 1
#cmakedefine HAVE_STRERROR 1
#cmakedefine HAVE_STRERROR_L 1
#cmakedefine HAVE_FORK 1
#cmakedefine HAVE_EXECVP 1
#cmakedefine HAVE_CTERMID 1
#cmakedefine HAVE_CLOCK_GETTIME 1
#cmakedefine HAVE_STRING_H 1
#cmakedefine HAVE_STRINGS_H 1
#cmakedefine HAVE_SYS_IOCTL_H 1
#cmakedefine HAVE_SYS_RESOURCE_H 1
Expand Down Expand Up @@ -149,9 +144,7 @@
#define PKGLIBDIR "@CMAKE_INSTALL_LIBDIR@/@PROJECT_NAME@"

// CMake leaves it emtpy for non-existing type. Autoconf sets it to 0.
#define SIZEOF_LONG (@SIZEOF_LONG@+0)
#define SIZEOF_OFF_T (@SIZEOF_OFF_T@+0)
#define SIZEOF_OFF64_T (@SIZEOF_OFF64_T@+0)

#cmakedefine STDERR_FILENO @STDERR_FILENO@
#cmakedefine STDIN_FILENO @STDIN_FILENO@
Expand Down
10 changes: 5 additions & 5 deletions src/common.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
common: misc stuff... audio flush, status display...
copyright ?-2022 by the mpg123 project
copyright ?-2023 by the mpg123 project
free software under the terms of the LGPL 2.1
see COPYING and AUTHORS files in distribution or http://mpg123.org
Expand Down Expand Up @@ -320,12 +320,12 @@ void print_stat(mpg123_handle *fr, long offset, out123_handle *ao, int draw_bar
}
/* Taking pains to properly size the frame number fields. */
len = snprintf( framefmt, sizeof(framefmt)
, "%%0%d"OFF_P, (int)log10(frame+rframes)+1 );
, "%%0%d" PRIiMAX, (int)log10(frame+rframes)+1 );
if(len < 0 || len >= sizeof(framefmt))
memcpy(framefmt, "%05"OFF_P, sizeof("%05"OFF_P));
snprintf( framestr[0], sizeof(framestr[0])-1, framefmt, (off_p)frame);
memcpy(framefmt, "%05" PRIiMAX, sizeof("%05" PRIiMAX));
snprintf( framestr[0], sizeof(framestr[0])-1, framefmt, (intmax_t)frame);
framestr[0][sizeof(framestr[0])-1] = 0;
snprintf( framestr[1], sizeof(framestr[1])-1, framefmt, (off_p)rframes);
snprintf( framestr[1], sizeof(framestr[1])-1, framefmt, (intmax_t)rframes);
framestr[1][sizeof(framestr[1])-1] = 0;
/* Now start with the state line. */
memset(line, 0, linelen+1); /* Always one zero more. */
Expand Down
47 changes: 0 additions & 47 deletions src/compat/compat.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,8 @@

#include <errno.h>

#ifdef HAVE_STDLIB_H
/* realloc, size_t */
#include <stdlib.h>
#endif

#include <stddef.h>

Expand All @@ -48,16 +46,10 @@
#ifdef HAVE_SYS_TYPES_H
#include <sys/types.h>
#endif
#ifdef HAVE_INTTYPES_H
#include <inttypes.h>
#endif
#ifdef HAVE_STDINT_H
#include <stdint.h>
#endif
/* We want SIZE_MAX, etc. */
#ifdef HAVE_LIMITS_H
#include <limits.h>
#endif

#ifndef SIZE_MAX
#define SIZE_MAX ((size_t)-1)
Expand Down Expand Up @@ -85,26 +77,11 @@
#define INT32_MIN (-INT32_MAX - 1)
#endif

#ifndef OFF_MAX
#undef OFF_MIN
#if SIZEOF_OFF_T == 4
#define OFF_MAX INT32_MAX
#define OFF_MIN INT32_MIN
#elif SIZEOF_OFF_T == 8
#define OFF_MAX INT64_MAX
#define OFF_MIN INT64_MIN
#else
#error "Unexpected width of off_t."
#endif
#endif

// Add two values (themselves assumed to be < limit), saturating to given limit.
#define SATURATE_ADD(inout, add, limit) inout = (limit-add >= inout) ? inout+add : limit;
#define SATURATE_SUB(inout, sub, limit) inout = (limit+sub >= inout) ? inout-sub : limit;

#ifdef HAVE_STRING_H
#include <string.h>
#endif
#ifdef HAVE_STRINGS_H
#include <strings.h>
#endif
Expand Down Expand Up @@ -150,29 +127,6 @@ const char *INT123_strerror(int errnum);
and returns NULL on NULL input instead of crashing. */
char* INT123_compat_strdup(const char *s);

/* If we have the size checks enabled, try to derive some sane printfs.
Simple start: Use max integer type and format if long is not big enough.
I am hesitating to use %ll without making sure that it's there... */
#if (defined SIZEOF_OFF_T) && (SIZEOF_OFF_T > SIZEOF_LONG) && (defined PRIiMAX)
# define OFF_P PRIiMAX
typedef intmax_t off_p;
#else
# define OFF_P "li"
typedef long off_p;
#endif

#if (defined SIZEOF_SIZE_T) && (SIZEOF_SIZE_T > SIZEOF_LONG) && (defined PRIuMAX) && (defined PRIiMAX)
# define SIZE_P PRIuMAX
typedef uintmax_t size_p;
# define SSIZE_P PRIiMAX
typedef intmax_t ssize_p;
#else
# define SIZE_P "lu"
typedef unsigned long size_p;
# define SSIZE_P "ld"
typedef long ssize_p;
#endif

/* Get an environment variable, possibly converted to UTF-8 from wide string.
The return value is a copy that you shall free. */
char *INT123_compat_getenv(const char* name);
Expand Down Expand Up @@ -200,7 +154,6 @@ FILE* INT123_compat_fdopen(int fd, const char *mode);
int INT123_compat_close(int infd);
int INT123_compat_fclose(FILE* stream);


/**
* Setting binary mode on a descriptor, where necessary.
* We do not bother with errors. This has to work.
Expand Down
4 changes: 2 additions & 2 deletions src/control_generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ void generic_sendstat (mpg123_handle *fr)
double current_seconds, seconds_left;

if(!position_info(fr, 0, ao, &current_frame, &frames_left, &current_seconds, &seconds_left, NULL, NULL))
generic_sendmsg("F %"OFF_P" %"OFF_P" %3.2f %3.2f", (off_p)current_frame, (off_p)frames_left, current_seconds, seconds_left);
generic_sendmsg("F %" PRIiMAX " %" PRIiMAX " %3.2f %3.2f", (intmax_t)current_frame, (intmax_t)frames_left, current_seconds, seconds_left);
else
{
sendstat_disabled = TRUE;
Expand Down Expand Up @@ -854,7 +854,7 @@ int control_generic (mpg123_handle *fr)
newpos = mpg123_tell(fr);
if(newpos <= oldpos) mpg123_meta_free(fr);

generic_sendmsg("K %"OFF_P, (off_p)newpos);
generic_sendmsg("K %" PRIiMAX, (intmax_t)newpos);
continue;
}
/* JUMP */
Expand Down
7 changes: 3 additions & 4 deletions src/libmpg123/frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,12 @@ int INT123_frame_outbuffer(mpg123_handle *fr)
{
fr->err = MPG123_BAD_BUFFER;
if(NOQUIET)
merror( "have external buffer of size %"SIZE_P", need %"SIZE_P
, (size_p)fr->buffer.size, (size_p)size );
merror("have external buffer of size %zu, need %zu", fr->buffer.size, size);
return MPG123_ERR;
}
}

debug1("need frame buffer of %"SIZE_P, (size_p)size);
debug1("need frame buffer of %zu", size);
if(fr->buffer.rdata != NULL && fr->buffer.size != size)
{
free(fr->buffer.rdata);
Expand All @@ -222,7 +221,7 @@ int INT123_frame_outbuffer(mpg123_handle *fr)

int attribute_align_arg mpg123_replace_buffer(mpg123_handle *mh, void *data, size_t size)
{
debug2("replace buffer with %p size %"SIZE_P, data, (size_p)size);
debug2("replace buffer with %p size %zu", data, size);
if(mh == NULL) return MPG123_BAD_HANDLE;
/* Will accept any size, the error comes later... */
if(data == NULL)
Expand Down
4 changes: 2 additions & 2 deletions src/libmpg123/gapless.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ static void frame_buffercheck(mpg123_handle *fr)
}
if(VERBOSE3)
fprintf(stderr, "\nNote: Cut frame %" PRIi64 " buffer on end of stream to %"
PRIi64 " samples, fill now %"SIZE_P" bytes.\n"
, fr->num, (fr->num == fr->lastframe ? fr->lastoff : 0), (size_p)fr->buffer.fill);
PRIi64 " samples, fill now %zu bytes.\n"
, fr->num, (fr->num == fr->lastframe ? fr->lastoff : 0), fr->buffer.fill);
}

/* The first interesting frame: Skip some leading samples. */
Expand Down
8 changes: 4 additions & 4 deletions src/libmpg123/id3.c
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ static void store_id3_text(mpg123_string *sb, unsigned char *source, size_t sour
}
memcpy(sb->p, source, source_size);
sb->fill = source_size;
debug1("stored undecoded ID3 text of size %"SIZE_P, (size_p)source_size);
debug1("stored undecoded ID3 text of size %zu", source_size);
return;
}

Expand Down Expand Up @@ -567,7 +567,7 @@ static void process_comment(mpg123_handle *fr, enum frame_types tt, unsigned cha

if(realsize < (size_t)(descr-realdata))
{
if(NOQUIET) error1("Invalid frame size of %"SIZE_P" (too small for anything).", (size_p)realsize);
if(NOQUIET) error1("Invalid frame size of %zu (too small for anything).", realsize);
return;
}
if(encoding > mpg123_id3_enc_max)
Expand Down Expand Up @@ -622,8 +622,8 @@ static void process_comment(mpg123_handle *fr, enum frame_types tt, unsigned cha

if(VERBOSE4) /* Do _not_ print the verbatim text: The encoding might be funny! */
{
fprintf(stderr, "Note: ID3 comm/uslt desc of length %"SIZE_P".\n", (size_p)xcom->description.fill);
fprintf(stderr, "Note: ID3 comm/uslt text of length %"SIZE_P".\n", (size_p)xcom->text.fill);
fprintf(stderr, "Note: ID3 comm/uslt desc of length %zu.\n", xcom->description.fill);
fprintf(stderr, "Note: ID3 comm/uslt text of length %zu.\n", xcom->text.fill);
}
/* Look out for RVA info only when we really deal with a straight comment. */
if(tt == comment && localcom.description.fill > 0)
Expand Down
2 changes: 1 addition & 1 deletion src/libmpg123/index.c
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ int INT123_fi_set(struct frame_index *fi, int64_t *offsets, int64_t step, size_t

void INT123_fi_reset(struct frame_index *fi)
{
debug1("reset with size %"SIZE_P, (size_p)fi->size);
debug1("reset with size %zu", fi->size);
fi->fill = 0;
fi->step = 1;
fi->next = fi_next(fi);
Expand Down
13 changes: 13 additions & 0 deletions src/libmpg123/lfs_wrap.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@
#include <sys/stat.h>
#include <fcntl.h>

#ifndef OFF_MAX
#undef OFF_MIN
#if SIZEOF_OFF_T == 4
#define OFF_MAX INT32_MAX
#define OFF_MIN INT32_MIN
#elif SIZEOF_OFF_T == 8
#define OFF_MAX INT64_MAX
#define OFF_MIN INT64_MIN
#else
#error "Unexpected width of off_t."
#endif
#endif

// A paranoid check that someone did not define a wrong SIZEOF_OFF_T at configure time.
typedef unsigned char MPG123_STATIC_ASSERT[(SIZEOF_OFF_T == sizeof(off_t)) ? 1 : -1];

Expand Down
5 changes: 3 additions & 2 deletions src/libmpg123/libmpg123.c
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,8 @@ static void decode_the_frame(mpg123_handle *fr)
if(fr->buffer.fill < needed_bytes)
{
if(VERBOSE2)
fprintf(stderr, "Note: broken frame %li, filling up with %"SIZE_P" zeroes, from %"SIZE_P"\n", (long)fr->num, (size_p)(needed_bytes-fr->buffer.fill), (size_p)fr->buffer.fill);
fprintf( stderr, "Note: broken frame %li, filling up with %zu zeroes, from %zu\n"
, (long)fr->num, (needed_bytes-fr->buffer.fill), fr->buffer.fill );

/*
One could do a loop with individual samples instead... but zero is zero
Expand All @@ -931,7 +932,7 @@ static void decode_the_frame(mpg123_handle *fr)
else
{
if(NOQUIET)
error2("I got _more_ bytes than expected (%"SIZE_P" / %"SIZE_P"), that should not be possible!", (size_p)fr->buffer.fill, (size_p)needed_bytes);
error2("I got _more_ bytes than expected (%zu / %zu), that should not be possible!", fr->buffer.fill, needed_bytes);
}
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion src/libmpg123/mpg123.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
*/
#define MPG123_API_VERSION 48
/** library patch level at client build time */
#define MPG123_PATCHLEVEL 1
#define MPG123_PATCHLEVEL 2

#ifndef MPG123_EXPORT
/** Defines needed for MS Visual Studio(tm) DLL builds.
Expand Down
Loading

0 comments on commit 72157ad

Please sign in to comment.