Prefer signed integer types in module code

Generally speaking, at the C level the Emacs source code prefers
signed types like ‘ptrdiff_t’ to unsigned types like ‘size_t’,
partly to avoid the usual signedness confusion when comparing values.
Change the module API to follow this convention.
Use ‘int’ for small values that can’t exceed INT_MAX.
* modules/mod-test/mod-test.c (Fmod_test_globref_make)
(Fmod_test_string_a_to_b, Fmod_test_vector_fill)
(Fmod_test_vector_eq):
* src/emacs-module.c (struct emacs_value_frame)
(module_make_global_ref, module_free_global_ref)
(module_copy_string_contents, module_make_string)
(module_vec_set, module_vec_get, module_vec_size):
* src/emacs-module.h (struct emacs_runtime, struct emacs_env_25):
* src/lread.c (suffix_p):
Prefer signed to unsigned integer types.
This commit is contained in:
Paul Eggert 2015-11-19 15:01:26 -08:00
parent d9b300af5c
commit 92949781eb
4 changed files with 33 additions and 45 deletions

View file

@ -117,7 +117,7 @@ Fmod_test_globref_make (emacs_env *env, int nargs, emacs_value args[],
{ {
/* Make a big string and make it global. */ /* Make a big string and make it global. */
char str[26 * 100]; char str[26 * 100];
for (size_t i = 0; i < sizeof str; i++) for (int i = 0; i < sizeof str; i++)
str[i] = 'a' + (i % 26); str[i] = 'a' + (i % 26);
/* We don't need to null-terminate str. */ /* We don't need to null-terminate str. */
@ -133,14 +133,14 @@ Fmod_test_string_a_to_b (emacs_env *env, int nargs, emacs_value args[],
void *data) void *data)
{ {
emacs_value lisp_str = args[0]; emacs_value lisp_str = args[0];
size_t size = 0; ptrdiff_t size = 0;
char * buf = NULL; char * buf = NULL;
env->copy_string_contents (env, lisp_str, buf, &size); env->copy_string_contents (env, lisp_str, buf, &size);
buf = malloc (size); buf = malloc (size);
env->copy_string_contents (env, lisp_str, buf, &size); env->copy_string_contents (env, lisp_str, buf, &size);
for (size_t i = 0; i + 1 < size; i++) for (ptrdiff_t i = 0; i + 1 < size; i++)
if (buf[i] == 'a') if (buf[i] == 'a')
buf[i] = 'b'; buf[i] = 'b';
@ -191,8 +191,8 @@ Fmod_test_vector_fill (emacs_env *env, int nargs, emacs_value args[], void *data
{ {
emacs_value vec = args[0]; emacs_value vec = args[0];
emacs_value val = args[1]; emacs_value val = args[1];
size_t size = env->vec_size (env, vec); ptrdiff_t size = env->vec_size (env, vec);
for (size_t i = 0; i < size; i++) for (ptrdiff_t i = 0; i < size; i++)
env->vec_set (env, vec, i, val); env->vec_set (env, vec, i, val);
return env->intern (env, "t"); return env->intern (env, "t");
} }
@ -205,8 +205,8 @@ Fmod_test_vector_eq (emacs_env *env, int nargs, emacs_value args[], void *data)
{ {
emacs_value vec = args[0]; emacs_value vec = args[0];
emacs_value val = args[1]; emacs_value val = args[1];
size_t size = env->vec_size (env, vec); ptrdiff_t size = env->vec_size (env, vec);
for (size_t i = 0; i < size; i++) for (ptrdiff_t i = 0; i < size; i++)
if (!env->eq (env, env->vec_get (env, vec, i), val)) if (!env->eq (env, env->vec_get (env, vec, i), val))
return env->intern (env, "nil"); return env->intern (env, "nil");
return env->intern (env, "t"); return env->intern (env, "t");

View file

@ -78,7 +78,7 @@ struct emacs_value_frame
struct emacs_value_tag objects[value_frame_size]; struct emacs_value_tag objects[value_frame_size];
/* Index of the next free value in `objects'. */ /* Index of the next free value in `objects'. */
size_t offset; int offset;
/* Pointer to next frame, if any. */ /* Pointer to next frame, if any. */
struct emacs_value_frame *next; struct emacs_value_frame *next;
@ -263,13 +263,13 @@ module_make_global_ref (emacs_env *env, emacs_value ref)
{ {
Lisp_Object value = HASH_VALUE (h, i); Lisp_Object value = HASH_VALUE (h, i);
eassert (NATNUMP (value)); eassert (NATNUMP (value));
EMACS_UINT refcount = XFASTINT (value); EMACS_INT refcount = XFASTINT (value) + 1;
if (refcount >= MOST_POSITIVE_FIXNUM) if (refcount > MOST_POSITIVE_FIXNUM)
{ {
module_non_local_exit_signal_1 (env, Qoverflow_error, Qnil); module_non_local_exit_signal_1 (env, Qoverflow_error, Qnil);
return NULL; return NULL;
} }
XSETFASTINT (value, refcount + 1); value = make_natnum (refcount);
set_hash_value_slot (h, i, value); set_hash_value_slot (h, i, value);
} }
else else
@ -297,15 +297,15 @@ module_free_global_ref (emacs_env *env, emacs_value ref)
{ {
Lisp_Object value = HASH_VALUE (h, i); Lisp_Object value = HASH_VALUE (h, i);
eassert (NATNUMP (value)); eassert (NATNUMP (value));
EMACS_UINT refcount = XFASTINT (value); EMACS_INT refcount = XFASTINT (value) - 1;
eassert (refcount > 0); if (refcount > 0)
if (refcount > 1)
{ {
XSETFASTINT (value, refcount - 1); value = make_natnum (refcount - 1);
set_hash_value_slot (h, i, value); set_hash_value_slot (h, i, value);
} }
else else
{ {
eassert (refcount == 0);
hash_remove_from_table (h, value); hash_remove_from_table (h, value);
} }
} }
@ -503,7 +503,7 @@ module_make_float (emacs_env *env, double d)
static bool static bool
module_copy_string_contents (emacs_env *env, emacs_value value, char *buffer, module_copy_string_contents (emacs_env *env, emacs_value value, char *buffer,
size_t *length) ptrdiff_t *length)
{ {
check_main_thread (); check_main_thread ();
eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return); eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
@ -515,7 +515,7 @@ module_copy_string_contents (emacs_env *env, emacs_value value, char *buffer,
return false; return false;
} }
size_t raw_size = SBYTES (lisp_str); ptrdiff_t raw_size = SBYTES (lisp_str);
/* Emacs internal encoding is more-or-less UTF8, let's assume utf8 /* Emacs internal encoding is more-or-less UTF8, let's assume utf8
encoded emacs string are the same byte size. */ encoded emacs string are the same byte size. */
@ -536,7 +536,7 @@ module_copy_string_contents (emacs_env *env, emacs_value value, char *buffer,
} }
static emacs_value static emacs_value
module_make_string (emacs_env *env, const char *str, size_t length) module_make_string (emacs_env *env, const char *str, ptrdiff_t length)
{ {
check_main_thread (); check_main_thread ();
eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return); eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
@ -609,7 +609,7 @@ module_set_user_finalizer (emacs_env *env, emacs_value uptr,
} }
static void static void
module_vec_set (emacs_env *env, emacs_value vec, size_t i, emacs_value val) module_vec_set (emacs_env *env, emacs_value vec, ptrdiff_t i, emacs_value val)
{ {
check_main_thread (); check_main_thread ();
eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return); eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
@ -633,41 +633,29 @@ module_vec_set (emacs_env *env, emacs_value vec, size_t i, emacs_value val)
} }
static emacs_value static emacs_value
module_vec_get (emacs_env *env, emacs_value vec, size_t i) module_vec_get (emacs_env *env, emacs_value vec, ptrdiff_t i)
{ {
/* Type of ASIZE (lvec) is ptrdiff_t, make sure it fits. */
verify (PTRDIFF_MAX <= SIZE_MAX);
check_main_thread (); check_main_thread ();
eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return); eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
if (i > MOST_POSITIVE_FIXNUM)
{
module_non_local_exit_signal_1 (env, Qoverflow_error, Qnil);
return NULL;
}
Lisp_Object lvec = value_to_lisp (vec); Lisp_Object lvec = value_to_lisp (vec);
if (! VECTORP (lvec)) if (! VECTORP (lvec))
{ {
module_wrong_type (env, Qvectorp, lvec); module_wrong_type (env, Qvectorp, lvec);
return NULL; return NULL;
} }
/* Prevent error-prone comparison between types of different signedness. */ ptrdiff_t size = ASIZE (lvec);
size_t size = ASIZE (lvec);
eassert (size >= 0); eassert (size >= 0);
if (i >= size) if (! (0 <= i && i < size))
{ {
if (i > MOST_POSITIVE_FIXNUM)
i = (size_t) MOST_POSITIVE_FIXNUM;
module_args_out_of_range (env, lvec, make_number (i)); module_args_out_of_range (env, lvec, make_number (i));
return NULL; return NULL;
} }
return lisp_to_value (env, AREF (lvec, i)); return lisp_to_value (env, AREF (lvec, i));
} }
static size_t static ptrdiff_t
module_vec_size (emacs_env *env, emacs_value vec) module_vec_size (emacs_env *env, emacs_value vec)
{ {
/* Type of ASIZE (lvec) is ptrdiff_t, make sure it fits. */
verify (PTRDIFF_MAX <= SIZE_MAX);
check_main_thread (); check_main_thread ();
eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return); eassert (module_non_local_exit_check (env) == emacs_funcall_exit_return);
Lisp_Object lvec = value_to_lisp (vec); Lisp_Object lvec = value_to_lisp (vec);
@ -947,7 +935,7 @@ void mark_modules (void)
for (struct emacs_value_frame *frame = &env->priv.storage.initial; for (struct emacs_value_frame *frame = &env->priv.storage.initial;
frame != NULL; frame != NULL;
frame = frame->next) frame = frame->next)
for (size_t i = 0; i < frame->offset; ++i) for (int i = 0; i < frame->offset; ++i)
mark_object (frame->objects[i].v); mark_object (frame->objects[i].v);
} }
} }

View file

@ -46,7 +46,7 @@ enum emacs_arity { emacs_variadic_function = -2 };
struct emacs_runtime struct emacs_runtime
{ {
/* Structure size (for version checking). */ /* Structure size (for version checking). */
size_t size; ptrdiff_t size;
/* Private data; users should not touch this. */ /* Private data; users should not touch this. */
struct emacs_runtime_private *private_members; struct emacs_runtime_private *private_members;
@ -82,7 +82,7 @@ enum emacs_funcall_exit
struct emacs_env_25 struct emacs_env_25
{ {
/* Structure size (for version checking). */ /* Structure size (for version checking). */
size_t size; ptrdiff_t size;
/* Private data; users should not touch this. */ /* Private data; users should not touch this. */
struct emacs_env_private *private_members; struct emacs_env_private *private_members;
@ -165,11 +165,11 @@ struct emacs_env_25
bool (*copy_string_contents) (emacs_env *env, bool (*copy_string_contents) (emacs_env *env,
emacs_value value, emacs_value value,
char *buffer, char *buffer,
size_t *size_inout); ptrdiff_t *size_inout);
/* Create a Lisp string from a utf8 encoded string. */ /* Create a Lisp string from a utf8 encoded string. */
emacs_value (*make_string) (emacs_env *env, emacs_value (*make_string) (emacs_env *env,
const char *contents, size_t length); const char *contents, ptrdiff_t length);
/* Embedded pointer type. */ /* Embedded pointer type. */
emacs_value (*make_user_ptr) (emacs_env *env, emacs_value (*make_user_ptr) (emacs_env *env,
@ -186,12 +186,12 @@ struct emacs_env_25
void (*fin) (void *) EMACS_NOEXCEPT); void (*fin) (void *) EMACS_NOEXCEPT);
/* Vector functions. */ /* Vector functions. */
emacs_value (*vec_get) (emacs_env *env, emacs_value vec, size_t i); emacs_value (*vec_get) (emacs_env *env, emacs_value vec, ptrdiff_t i);
void (*vec_set) (emacs_env *env, emacs_value vec, size_t i, void (*vec_set) (emacs_env *env, emacs_value vec, ptrdiff_t i,
emacs_value val); emacs_value val);
size_t (*vec_size) (emacs_env *env, emacs_value vec); ptrdiff_t (*vec_size) (emacs_env *env, emacs_value vec);
}; };
#ifdef __cplusplus #ifdef __cplusplus

View file

@ -979,8 +979,8 @@ This uses the variables `load-suffixes' and `load-file-rep-suffixes'. */)
static bool static bool
suffix_p (Lisp_Object string, const char *suffix) suffix_p (Lisp_Object string, const char *suffix)
{ {
size_t suffix_len = strlen (suffix); ptrdiff_t suffix_len = strlen (suffix);
size_t string_len = SBYTES (string); ptrdiff_t string_len = SBYTES (string);
return string_len >= suffix_len && !strcmp (SSDATA (string) + string_len - suffix_len, suffix); return string_len >= suffix_len && !strcmp (SSDATA (string) + string_len - suffix_len, suffix);
} }