View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000566 | LDMud 3.5 | Implementation | public | 2008-09-09 14:16 | 2009-01-06 07:23 |
| Reporter | zesstra | Assigned To | zesstra | ||
| Priority | normal | Severity | minor | Reproducibility | N/A |
| Status | assigned | Resolution | open | ||
| Summary | 0000566: Replace Code-Defines by static inline functions | ||||
| Description | There are a lot of macros in the driver for convenience, which evaluate to some value or do some little things and are often called, e.g. #define mstrsize(s) \ ((s)->size) from mstrings.c. I would like to change these defines to static inline functions. Modern compilers are good at inlining (they would even inline without requesting it by 'inline') and the runtime behaviour is identical to the preprocessor macros. But functions have (at least) three big advantages: a) they are visible to the compiler and thus the debugger. b) for debugging it is possible to disable inlining and/or set breakpoints (which you can't in case of preprocessor macros). c) type safety: functions have explicit types for their arguments and return values and the compiler can check them, which it can't in case of macros. Additionally it is much easier for the programmer to keep track of types and the code is less error-prone. Example for c): If I don't know the type of s->size, I have to do a tedious search for the type and definition of s. A function: static inline size_t mstrsize(string_t *s) {return s->size;} is much easier in this respect and the compiler can issue warning if foo in mstrsize(foo) is not a pointer to string_t or if len in len=mstrsize(foo) is of an incompatible type. I suggest to change all macros, which do not change local variables bit by bit to static inlines functions. side remark: even for numerical constants defines are not really needed and have again the disadvantage that the define in not visible in the debugger. If you use 'static const long LIMIT 2' the generated code is _exactly the same_ as if you use '#define LIMIT 2L'. (static ensures that the compiler doesn't emit a symbol LIMIT in the data segment.) So I would even recommend using consts for numerical constants as well, there is no speed or memory penalty (in modern compilers). | ||||
| Additional Information | http://www.fefe.de/know-your-compiler.pdf http://www.greenend.org.uk/rjk/2003/03/inline.html http://gcc.gnu.org/onlinedocs/gcc/Inline.html | ||||
| Tags | No tags attached. | ||||
| Attached Files | comm.c.diff (3,720 bytes)
Index: src/comm.c
===================================================================
--- src/comm.c (Revision 2414)
+++ src/comm.c (Arbeitskopie)
@@ -394,22 +394,26 @@
/* Tables with the telnet statemachine handlers.
*/
-#define TS_DATA 0
-#define TS_IAC 1
-#define TS_WILL 2
-#define TS_WONT 3
-#define TS_DO 4
-#define TS_DONT 5
-#define TS_SB 6
-#define TS_SB_IAC 7
-#define TS_READY 8
-#define TS_SYNCH 9
-#define TS_INVALID 10
+enum telnet_states {
+ TS_DATA = 0,
+ TS_IAC,
+ TS_WILL,
+ TS_WONT,
+ TS_DO,
+ TS_DONT,
+ TS_SB,
+ TS_SB_IAC,
+ TS_READY,
+ TS_SYNCH,
+ TS_INVALID
+};
/* Telnet states
*/
-#define TN_START_VALID(x) ((x & ~1) == TS_SB)
+inline int TN_START_VALID(int x) {
+ return (x & ~TS_IAC) == TS_SB;
+}
/* --- Misc --- */
@@ -517,17 +521,14 @@
# define s6_addr32 __u6_addr.__u6_addr32
#endif
-#ifndef CREATE_IPV6_MAPPED
+inline void CREATE_IPV6_MAPPED(struct in_addr *v6, int32 v4) {
+ v6->s6_addr32[0] = 0;
+ v6->s6_addr32[1] = 0;
+ v6->s6_addr32[2] = 0x0000ffff;
+ v6->s6_addr32[2] = 0xffff0000;
+ v6->s6_addr32[3] = v4;
+}
-#define CREATE_IPV6_MAPPED(v6,v4) \
- ((v6).s6_addr32[0] = 0, \
- (v6).s6_addr32[1] = 0, \
- (v6).s6_addr32[2] = 0x0000ffff, \
- (v6).s6_addr32[2] = 0xffff0000, \
- (v6).s6_addr32[3] = v4 )
-
-#endif
-
/* These are the typical IPv6 structures - we use them transparently.
*
* --- arpa/inet.h ---
@@ -2754,7 +2755,7 @@
#ifndef USE_IPV6
net_addr.s_addr = naddr;
#else
- CREATE_IPV6_MAPPED(net_addr, naddr);
+ CREATE_IPV6_MAPPED(&net_addr, naddr);
#endif
add_ip_entry(net_addr, rp+12);
}
@@ -6400,7 +6401,7 @@
h = gethostbyname2(hostname, AF_INET);
if(h)
{
- CREATE_IPV6_MAPPED(addr.sin6_addr, *((u_int32_t *)h->h_addr_list[0]));
+ CREATE_IPV6_MAPPED(&addr.sin6_addr, *((u_int32_t *)h->h_addr_list[0]));
con = connect(sock, (struct sockaddr *) &addr, sizeof(addr));
}
}
@@ -7129,7 +7130,7 @@
if (hp->h_addrtype == AF_INET)
{
- CREATE_IPV6_MAPPED(name.sin_addr, (u_int32_t)hp->h_addr_list[0]);
+ CREATE_IPV6_MAPPED(&name.sin_addr, (u_int32_t)hp->h_addr_list[0]);
}
name.sin_family = AF_INET6;
#endif /* USE_IPV6 */
@@ -8315,6 +8316,12 @@
return sp;
} /* f_query_mud_port() */
+inline void TRANSLATE(char c, int i, int length, string_t *rc, unsigned int bitno) {
+ if (c & (1 << bitno))
+ get_txt(rc)[length++] = (char)(i * 8 + bitno);
+}
+
+
/*-------------------------------------------------------------------------*/
static void
get_charset (svalue_t * sp, p_int mode, char charset[32])
@@ -8385,19 +8392,14 @@
{
char c = charset[i];
-#define TRANSLATE(bitno) \
- if (c & (1 << bitno)) \
- get_txt(rc)[length++] = (char)(i * 8 + bitno);
-
- TRANSLATE(0);
- TRANSLATE(1);
- TRANSLATE(2);
- TRANSLATE(3);
- TRANSLATE(4);
- TRANSLATE(5);
- TRANSLATE(6);
- TRANSLATE(7);
-#undef TRANSLATE
+ TRANSLATE(c, i, length, rc, 0);
+ TRANSLATE(c, i, length, rc, 1);
+ TRANSLATE(c, i, length, rc, 2);
+ TRANSLATE(c, i, length, rc, 3);
+ TRANSLATE(c, i, length, rc, 4);
+ TRANSLATE(c, i, length, rc, 5);
+ TRANSLATE(c, i, length, rc, 6);
+ TRANSLATE(c, i, length, rc, 7);
}
put_string(sp, rc);
alloc.diff (12,762 bytes)
Index: src/xalloc.c
===================================================================
--- src/xalloc.c (Revision 2414)
+++ src/xalloc.c (Arbeitskopie)
@@ -44,13 +44,38 @@
typedef struct { unsigned long counter, size; } t_stat;
/* A counter type for statistics and its functions: */
-#define count(a,b) { a.size+=(b); if ((b)<0) --a.counter; else ++a.counter; }
-#define count_up(a,b) { a.size+=(b); ++a.counter; }
-#define count_up_n(a,b,c) { a.size+=(b)*(c); a.counter+=(b); }
-#define count_add(a,b) { a.size+=(b); }
-#define count_back(a,b) { a.size-=(b); --a.counter; }
-#define count_back_n(a,b,c) { a.size-=(b)*(c); a.counter-=(b); }
+inline void count_add(t_stat *a, unsigned int b) {
+ a->size += b;
+}
+inline void count(t_stat *a, unsigned int b) {
+ count_add(a, b);
+ if (b < 0)
+ --a->counter;
+ else
+ ++a->counter;
+}
+
+inline void count_up(t_stat *a, unsigned int b) {
+ count_add(a, b);
+ ++a->counter;
+}
+
+inline void count_up_n(t_stat *a, unsigned int b, unsigned int c) {
+ count_add(a, b * c);
+ a->counter += b;
+}
+
+inline void count_back(t_stat *a, unsigned int b) {
+ count_add(a, -b);
+ --a->counter;
+}
+
+inline void count_back_n(t_stat *a, unsigned int b, unsigned int c) {
+ count_add(a, -(b * c));
+ a->counter -= b;
+}
+
typedef p_uint word_t;
/* Our 'word' type.
*/
@@ -555,9 +580,9 @@
p[XM_PC] = (word_t)inter_pc;
#endif
#ifdef NO_MEM_BLOCK_SIZE
- count_up(xalloc_stat, XM_OVERHEAD_SIZE);
+ count_up(&xalloc_stat, XM_OVERHEAD_SIZE);
#else
- count_up(xalloc_stat, mem_block_size(p));
+ count_up(&xalloc_stat, mem_block_size(p));
if (check_max_malloced())
return NULL;
#endif
@@ -576,9 +601,9 @@
{
word_t *q = (word_t*)p - XM_OVERHEAD;
#ifdef NO_MEM_BLOCK_SIZE
- count_back(xalloc_stat, XM_OVERHEAD_SIZE);
+ count_back(&xalloc_stat, XM_OVERHEAD_SIZE);
#else
- count_back(xalloc_stat, mem_block_size(q));
+ count_back(&xalloc_stat, mem_block_size(q));
#endif
mem_free(q);
}
@@ -673,8 +698,8 @@
#ifndef NO_MEM_BLOCK_SIZE
if (rc != NULL)
{
- count_back(xalloc_stat, old_size);
- count_up(xalloc_stat, mem_block_size(block));
+ count_back(&xalloc_stat, old_size);
+ count_up(&xalloc_stat, mem_block_size(block));
if (check_max_malloced())
return NULL;
}
@@ -749,8 +774,8 @@
if (t)
{
#ifndef NO_MEM_BLOCK_SIZE
- count_back(xalloc_stat, old_size);
- count_up(xalloc_stat, mem_block_size(t));
+ count_back(&xalloc_stat, old_size);
+ count_up(&xalloc_stat, mem_block_size(t));
if (check_max_malloced())
return NULL;
#endif
@@ -1262,7 +1287,7 @@
result = amalloc(size);
if (result)
{
- count_up(clib_alloc_stat, get_block_size(result));
+ count_up(&clib_alloc_stat, get_block_size(result));
}
return result;
@@ -1278,7 +1303,7 @@
{
if (ptr)
{
- count_back(clib_alloc_stat, get_block_size(ptr));
+ count_back(&clib_alloc_stat, get_block_size(ptr));
}
afree(ptr);
Index: src/slaballoc.c
===================================================================
--- src/slaballoc.c (Revision 2414)
+++ src/slaballoc.c (Arbeitskopie)
@@ -729,7 +729,7 @@
if (q[-M_OVERHEAD] & M_GC_FREE)
{
q[-M_OVERHEAD] &= ~M_GC_FREE;
- count_up(perm_alloc_stat, mem_block_total_size(q));
+ count_up(&perm_alloc_stat, mem_block_total_size(q));
}
} /* mem_mark_permanent() */
@@ -746,7 +746,7 @@
if (!(q[-M_OVERHEAD] & M_GC_FREE))
{
q[-M_OVERHEAD] |= (M_REF|M_GC_FREE);
- count_back(perm_alloc_stat, mem_block_total_size(q));
+ count_back(&perm_alloc_stat, mem_block_total_size(q));
}
} /* mem_mark_collectable() */
@@ -1513,8 +1513,8 @@
{
ulog2f("slaballoc: deallocate slab %x [%d]\n", slab, ix);
slabtable[ix].numSlabs--;
- count_back(small_slab_stat, SLAB_SIZE(slab, ix));
- count_back_n(small_free_stat, slabtable[ix].numBlocks, slab->size);
+ count_back(&small_slab_stat, SLAB_SIZE(slab, ix));
+ count_back_n(&small_free_stat, slabtable[ix].numBlocks, slab->size);
#ifdef MALLOC_EXT_STATISTICS
extstats[SIZE_INDEX(slab->size)].cur_free -= slabtable[ix].numBlocks;
extstats[EXTSTAT_SLABS].cur_free--;
@@ -1591,7 +1591,7 @@
ulog2f(" %d bytes -> [%d]\n", size, ix);
/* Update statistics */
- count_up(small_alloc_stat,size);
+ count_up(&small_alloc_stat,size);
#ifdef MALLOC_EXT_STATISTICS
extstats[SIZE_INDEX(size)].num_xalloc++;
@@ -1612,7 +1612,7 @@
block = slab->freeList;
slab->freeList = BLOCK_NEXT(slab->freeList);
- count_back(small_free_stat, slab->size);
+ count_back(&small_free_stat, slab->size);
#ifdef MALLOC_EXT_STATISTICS
extstats[ix].cur_free--;
@@ -1701,7 +1701,7 @@
}
slabtable[ix].numFreeSlabs--;
- count_back(small_slab_free_stat, SLAB_SIZE(slab, ix));
+ count_back(&small_slab_free_stat, SLAB_SIZE(slab, ix));
#ifdef MALLOC_EXT_STATISTICS
extstats[EXTSTAT_SLABS].cur_free--;
#endif /* MALLOC_EXT_STATISTICS */
@@ -1733,8 +1733,8 @@
slab->size = size;
slabtable[ix].numSlabs++;
- count_up(small_slab_stat, slabSize);
- count_up_n(small_free_stat, numObjects, size);
+ count_up(&small_slab_stat, slabSize);
+ count_up_n(&small_free_stat, numObjects, size);
#ifdef MALLOC_EXT_STATISTICS
extstats[ix].cur_free += numObjects;
extstats[EXTSTAT_SLABS].num_xalloc++;
@@ -1781,7 +1781,7 @@
MADVISE(block, orig_size);
- count_back(small_free_stat, size);
+ count_back(&small_free_stat, size);
#ifdef MALLOC_EXT_STATISTICS
extstats[ix].cur_free--;
#endif /* MALLOC_EXT_STATISTICS */
@@ -1870,7 +1870,7 @@
/* It's a small block: put it into the slab's free list */
slab = (mslab_t*)(block - (block[M_SIZE] & M_MASK));
- count_back(small_alloc_stat, slab->size);
+ count_back(&small_alloc_stat, slab->size);
ix = SIZE_INDEX(slab->size);
ulog4f("slaballoc: -> slab %x [%d], freelist %x, %d free\n"
@@ -1933,7 +1933,7 @@
slab->freeList = block;
slab->numAllocated--;
- count_up(small_free_stat, slab->size);
+ count_up(&small_free_stat, slab->size);
/* If this slab is not the fresh slab, handle possible list movements.
*/
@@ -1993,7 +1993,7 @@
slabtable[ix].firstFree = slab;
slabtable[ix].numFreeSlabs++;
- count_up(small_slab_free_stat, SLAB_SIZE(slab, ix));
+ count_up(&small_slab_free_stat, SLAB_SIZE(slab, ix));
#ifdef MALLOC_EXT_STATISTICS
extstats[EXTSTAT_SLABS].cur_alloc--;
extstats[EXTSTAT_SLABS].cur_free++;
@@ -2314,7 +2314,7 @@
}
#endif
p = (struct free_block *)(ptr+M_OVERHEAD);
- count_back(large_free_stat, p->size);
+ count_back(&large_free_stat, p->size);
#ifdef MALLOC_EXT_STATISTICS
extstats[EXTSTAT_LARGE].cur_free--;
#endif /* MALLOC_EXT_STATISTICS */
@@ -2703,7 +2703,7 @@
* register choice
*/
r = (struct free_block *)(ptr+M_OVERHEAD);
- count_up(large_free_stat, size);
+ count_up(&large_free_stat, size);
#ifdef MALLOC_EXT_STATISTICS
extstats[EXTSTAT_LARGE].cur_free++;
extstat_update_max(extstats+EXTSTAT_LARGE);
@@ -3344,7 +3344,7 @@
{
mark_block(ptr+size);
*(ptr+size) &= ~M_GC_FREE; /* Hands off, GC! */
- count_up(large_wasted_stat, (*(ptr+size) & M_MASK) * SINT);
+ count_up(&large_wasted_stat, (*(ptr+size) & M_MASK) * SINT);
}
else
# endif
@@ -3360,7 +3360,7 @@
/* The block at ptr is all ours */
mark_block(ptr);
- count_up(large_alloc_stat, size);
+ count_up(&large_alloc_stat, size);
#ifdef MALLOC_EXT_STATISTICS
extstats[EXTSTAT_LARGE].num_xalloc++;
extstats[EXTSTAT_LARGE].cur_alloc++;
@@ -3388,7 +3388,7 @@
p = (word_t *) ptr;
p -= M_OVERHEAD;
size = p[M_LSIZE];
- count_back(large_alloc_stat, size);
+ count_back(&large_alloc_stat, size);
#ifdef MALLOC_EXT_STATISTICS
extstats[EXTSTAT_LARGE].num_xfree++;
extstats[EXTSTAT_LARGE].cur_alloc--;
@@ -3460,7 +3460,7 @@
}
*heap_start = 2;
*(heap_start+1) = PREV_BLOCK | M_MASK;
- count_up(large_wasted_stat, 2*SINT);
+ count_up(&large_wasted_stat, 2*SINT);
assert_stack_gap();
}
@@ -3468,7 +3468,7 @@
if ((int)brk((char *)heap_end + size) == -1)
return NULL;
- count_up(sbrk_stat, size);
+ count_up(&sbrk_stat, size);
heap_end = (word_t*)((char *)heap_end + size);
heap_end[-1] = THIS_BLOCK | M_MASK;
heap_end[-2] = M_MASK;
@@ -3535,7 +3535,7 @@
/* We can join with the existing heap */
p[overhead] &= ~PREV_BLOCK;
overlap = SINT;
- count_back(large_wasted_stat, overlap);
+ count_back(&large_wasted_stat, overlap);
}
else
{
@@ -3559,7 +3559,7 @@
heap_end = (word_t *)(block + size);
block -= overhead;
overlap = overhead * SINT;
- count_back(large_wasted_stat, overlap);
+ count_back(&large_wasted_stat, overlap);
}
else
{
@@ -3594,7 +3594,7 @@
/* Our block directly follows the one we found */
block -= overhead;
overlap += overhead * SINT;
- count_back(large_wasted_stat, overhead * SINT);
+ count_back(&large_wasted_stat, overhead * SINT);
}
else
{
@@ -3619,7 +3619,7 @@
/* Our block directly preceedes the next one */
*(next+1) &= ~PREV_BLOCK;
overlap += overhead * SINT;
- count_back(large_wasted_stat, overhead * SINT);
+ count_back(&large_wasted_stat, overhead * SINT);
}
else
{
@@ -3630,8 +3630,8 @@
}
}
- count_up(sbrk_stat, size);
- count_up(large_wasted_stat, overhead * SINT);
+ count_up(&sbrk_stat, size);
+ count_up(&large_wasted_stat, overhead * SINT);
*pExtra = overlap;
return block + SINT;
@@ -3681,7 +3681,7 @@
start[M_LSIZE] += wsize;
malloc_increment_size_success++;
malloc_increment_size_total += (start2 - start) - M_OVERHEAD;
- count_add(large_alloc_stat, wsize);
+ count_add(&large_alloc_stat, wsize);
return start2+M_LSIZE;
}
@@ -3699,7 +3699,7 @@
start[M_LSIZE] += wsize;
malloc_increment_size_success++;
malloc_increment_size_total += (start2 - start) - M_OVERHEAD;
- count_add(large_alloc_stat, wsize);
+ count_add(&large_alloc_stat, wsize);
return start2+M_LSIZE;
}
@@ -3950,7 +3950,7 @@
{
/* Unref'd small blocks are definitely lost */
success++;
- count_back(xalloc_stat, slab->size - (T_OVERHEAD * SINT));
+ count_back(&xalloc_stat, slab->size - (T_OVERHEAD * SINT));
dprintf2(gcollect_outfd, "freeing small block 0x%x (user 0x%x)"
, (p_uint)p, (p_uint)(p+M_OVERHEAD));
#ifdef MALLOC_TRACE
@@ -4007,7 +4007,7 @@
word_t size2, flags2;
success++;
- count_back(xalloc_stat, mem_block_size(p+ML_OVERHEAD));
+ count_back(&xalloc_stat, mem_block_size(p+ML_OVERHEAD));
#if defined(MALLOC_TRACE) || defined(MALLOC_LPC_TRACE)
dprintf1(gcollect_outfd, "freeing large block 0x%x", (p_uint)p);
#endif
@@ -4319,7 +4319,7 @@
while (NULL != (slab = slabtable[ix].firstFree))
{
slabtable[ix].firstFree = slab->next;
- count_back(small_slab_free_stat, SLAB_SIZE(slab, ix));
+ count_back(&small_slab_free_stat, SLAB_SIZE(slab, ix));
free_slab(slab, ix);
}
slabtable[ix].numFreeSlabs = 0;
@@ -4343,7 +4343,7 @@
else
slabtable[ix].firstFree = NULL;
slabtable[ix].numFreeSlabs--;
- count_back(small_slab_free_stat, SLAB_SIZE(slab, ix));
+ count_back(&small_slab_free_stat, SLAB_SIZE(slab, ix));
free_slab(slab, ix);
}
#ifdef DEBUG_MALLOC_ALLOCS
sprintf.c.diff (14,198 bytes)
Index: src/sprintf.c
===================================================================
--- src/sprintf.c (Revision 2414)
+++ src/sprintf.c (Arbeitskopie)
@@ -133,30 +133,32 @@
typedef unsigned int format_info;
-#define INFO_T 0xF
-#define INFO_T_ERROR 0x1
-#define INFO_T_NULL 0x2
-#define INFO_T_LPC 0x3
-#define INFO_T_QLPC 0x4
-#define INFO_T_STRING 0x5
-#define INFO_T_INT 0x6
-#define INFO_T_FLOAT 0x7
+enum format_info_t {
+ INFO_T = 0xF,
+ INFO_T_ERROR = 0x1,
+ INFO_T_NULL = 0x2,
+ INFO_T_LPC = 0x3,
+ INFO_T_QLPC = 0x4,
+ INFO_T_STRING = 0x5,
+ INFO_T_INT = 0x6,
+ INFO_T_FLOAT = 0x7,
-#define INFO_A 0x30 /* Right alignment */
-#define INFO_A_CENTRE 0x10
-#define INFO_A_LEFT 0x20
-#define INFO_A_JUSTIFY 0x30
+ INFO_A = 0x30, /* Right alignment */
+ INFO_A_CENTRE = 0x10,
+ INFO_A_LEFT = 0x20,
+ INFO_A_JUSTIFY = 0x30,
-#define INFO_PP 0xC0
-#define INFO_PP_SPACE 0x40
-#define INFO_PP_PLUS 0x80
+ INFO_PP = 0xC0,
+ INFO_PP_SPACE = 0x40,
+ INFO_PP_PLUS = 0x80,
-#define INFO_ARRAY 0x100
-#define INFO_COLS 0x200
-#define INFO_TABLE 0x400
+ INFO_ARRAY = 0x100,
+ INFO_COLS = 0x200,
+ INFO_TABLE = 0x400,
-#define INFO_PS_ZERO 0x800
-#define INFO_PS_KEEP 0x1000
+ INFO_PS_ZERO = 0x800,
+ INFO_PS_KEEP = 0x1000,
+};
/*-------------------------------------------------------------------------*/
@@ -166,27 +168,28 @@
/* The error handling */
-#define ERROR(x) (longjmp(st->error_jmp, (x)))
+enum format_err {
+ ERR_ID_NUMBER = 0xFFFF, /* Mask for the error number */
+ ERR_ARGUMENT = 0xFFFF0000, /* Mask for the arg number */
-#define ERR_ID_NUMBER 0xFFFF /* Mask for the error number */
-#define ERR_ARGUMENT 0xFFFF0000 /* Mask for the arg number */
+ ERR_BUFF_OVERFLOW = 0x1, /* buffer overflowed */
+ ERR_TO_FEW_ARGS = 0x2, /* more arguments spec'ed than passed */
+ ERR_INVALID_STAR = 0x3, /* invalid arg to * */
+ ERR_PREC_EXPECTED = 0x4, /* expected precision not found */
+ ERR_INVALID_FORMAT_STR = 0x5, /* error in format string */
+ ERR_INCORRECT_ARG = 0x6, /* invalid arg to %[idcxXs] */
+ ERR_CST_REQUIRES_FS = 0x7, /* field size not given for c/t */
+ ERR_UNDEFINED_TYPE = 0x8, /* undefined type found */
+ ERR_QUOTE_EXPECTED = 0x9, /* expected ' not found */
+ ERR_UNEXPECTED_EOS = 0xA, /* fs terminated unexpectedly */
+ ERR_NULL_PS = 0xB, /* pad string is null */
+ ERR_ARRAY_EXPECTED = 0xC, /* array expected */
+ ERR_NOMEM = 0xD, /* Out of memory */
+ ERR_SIZE_OVERFLOW = 0xE, /* Fieldsize/precision numeric overflow */
+};
-#define ERR_BUFF_OVERFLOW 0x1 /* buffer overflowed */
-#define ERR_TO_FEW_ARGS 0x2 /* more arguments spec'ed than passed */
-#define ERR_INVALID_STAR 0x3 /* invalid arg to * */
-#define ERR_PREC_EXPECTED 0x4 /* expected precision not found */
-#define ERR_INVALID_FORMAT_STR 0x5 /* error in format string */
-#define ERR_INCORRECT_ARG 0x6 /* invalid arg to %[idcxXs] */
-#define ERR_CST_REQUIRES_FS 0x7 /* field size not given for c/t */
-#define ERR_UNDEFINED_TYPE 0x8 /* undefined type found */
-#define ERR_QUOTE_EXPECTED 0x9 /* expected ' not found */
-#define ERR_UNEXPECTED_EOS 0xA /* fs terminated unexpectedly */
-#define ERR_NULL_PS 0xB /* pad string is null */
-#define ERR_ARRAY_EXPECTED 0xC /* array expected */
-#define ERR_NOMEM 0xD /* Out of memory */
-#define ERR_SIZE_OVERFLOW 0xE /* Fieldsize/precision numeric overflow */
+#define ERROR(x) (longjmp(st->error_jmp, (x)))
-
#define ERROR1(e,a) ERROR((e) | (a)<<16)
#define EXTRACT_ERR_ARGUMENT(i) ((i)>>16)
@@ -327,59 +330,59 @@
/*-------------------------------------------------------------------------*/
/* Macros */
-#define ADD_CHAR(x) {\
- if (st->bpos >= BUFF_SIZE) ERROR(ERR_BUFF_OVERFLOW); \
- if (x == '\n' && st->sppos != -1) st->bpos = st->sppos; \
- st->sppos = -1; \
- st->buff[st->bpos++] = x;\
+inline void ADD_CHAR(fmt_state_t *st, char x) {
+ if (st->bpos >= BUFF_SIZE) ERROR(ERR_BUFF_OVERFLOW);
+ if (x == '\n' && st->sppos != -1) st->bpos = st->sppos;
+ st->sppos = -1;
+ st->buff[st->bpos++] = x;
}
/* Add character <x> to the buffer.
*/
/*-------------------------------------------------------------------------*/
-#define ADD_STRN(s, n) { \
- if (st->bpos + n > BUFF_SIZE) ERROR(ERR_BUFF_OVERFLOW); \
- if (n >= 1 && (s)[0] == '\n' && st->sppos != -1) st->bpos = st->sppos; \
- st->sppos = -1; \
- memcpy(st->buff+st->bpos, (s), n);\
- st->bpos += n; \
+inline void ADD_STRN(fmt_state_t *st, const char *s, size_t n) {
+ if (st->bpos + n > BUFF_SIZE) ERROR(ERR_BUFF_OVERFLOW);
+ if (n >= 1 && s[0] == '\n' && st->sppos != -1) st->bpos = st->sppos;
+ st->sppos = -1;
+ memcpy(st->buff+st->bpos, s, n);
+ st->bpos += n;
}
/* Add the <n> characters from <s> to the buffer.
*/
/*-------------------------------------------------------------------------*/
-#define ADD_CHARN(c, n) { \
- /* n must not be negative! */ \
- if (st->bpos + n > BUFF_SIZE) ERROR(ERR_BUFF_OVERFLOW); \
- if (n >= 1 && c == '\n' && st->sppos != -1) st->bpos = st->sppos; \
- st->sppos = -1; \
- memset(st->buff+st->bpos, c, n); \
- st->bpos += n; \
+inline void ADD_CHARN(fmt_state_t *st, char c, size_t n) {
+ /* n must not be negative! */
+ if (st->bpos + n > BUFF_SIZE) ERROR(ERR_BUFF_OVERFLOW);
+ if (n >= 1 && c == '\n' && st->sppos != -1) st->bpos = st->sppos;
+ st->sppos = -1;
+ memset(st->buff+st->bpos, c, n);
+ st->bpos += n;
}
/* Add character <c> <n>-times to the buffer.
*/
/*-------------------------------------------------------------------------*/
-#define ADD_PADDING(pad, N) { \
- int n = (N); \
-\
- if (!pad[1]) { \
- ADD_CHARN(*pad, n) \
- } else { \
- int l; \
-\
- l = strlen(pad); \
- for (i=0; --n >= 0; ) { \
- if (pad[i] == '\\') \
- i++; \
- ADD_CHAR(pad[i]); \
- if (++i == l) \
- i = 0; \
- } \
- } \
+inline void ADD_PADDING(fmt_state_t *st, const char *pad, size_t N) {
+ int n = N;
+
+ if (!pad[1]) {
+ ADD_CHARN(st, *pad, n);
+ } else {
+ int i, l;
+
+ l = strlen(pad);
+ for (i=0; --n >= 0; ) {
+ if (pad[i] == '\\')
+ i++;
+ ADD_CHAR(st, pad[i]);
+ if (++i == l)
+ i = 0;
+ }
+ }
}
/* Add the padding string <pad> to the buffer, repeatedly if necessary,
@@ -1082,7 +1085,6 @@
*/
{
- int i;
size_t sppos;
int num_words; /* Number of words in the input */
int num_chars; /* Number of non-space characters in the input */
@@ -1149,7 +1151,7 @@
for (mark = pos ; pos < len && str[pos] != ' '; pos++) NOOP;
/* Add the word */
- ADD_STRN(str+mark, pos - mark);
+ ADD_STRN(st, str+mark, pos - mark);
num_words--;
if (pos >= len || num_words < 1)
@@ -1169,7 +1171,7 @@
else /* Randomly add one space */
padlength = min_spaces + (int)random_number(2);
sppos = st->bpos;
- ADD_PADDING(" ", padlength);
+ ADD_PADDING(st, " ", padlength);
st->sppos = sppos;
num_spaces -= padlength;
@@ -1189,7 +1191,6 @@
*/
{
- int i;
size_t sppos;
Bool is_space_pad;
@@ -1206,10 +1207,10 @@
case INFO_A_JUSTIFY:
case INFO_A_LEFT:
/* Also called for the last line of a justified block */
- ADD_STRN(str, len)
+ ADD_STRN(st, str, len);
if (is_space_pad)
sppos = st->bpos;
- ADD_PADDING(pad, fs - len)
+ ADD_PADDING(st, pad, fs - len);
if (is_space_pad)
st->sppos = sppos;
break;
@@ -1217,16 +1218,16 @@
case INFO_A_CENTRE:
if (finfo & INFO_PS_ZERO)
{
- ADD_PADDING("0", (fs - len + 1) >> 1)
+ ADD_PADDING(st, "0", (fs - len + 1) >> 1);
}
else
{
- ADD_PADDING(pad, (fs - len + 1) >> 1)
+ ADD_PADDING(st, pad, (fs - len + 1) >> 1);
}
- ADD_STRN(str, len)
+ ADD_STRN(st, str, len);
if (is_space_pad)
sppos = st->bpos;
- ADD_PADDING(pad, (fs - len) >> 1)
+ ADD_PADDING(st, pad, (fs - len) >> 1);
if (is_space_pad)
st->sppos = sppos;
break;
@@ -1236,13 +1237,13 @@
*/
if (finfo & INFO_PS_ZERO)
{
- ADD_PADDING("0", fs - len)
+ ADD_PADDING(st, "0", fs - len);
}
else
{
- ADD_PADDING(pad, fs - len)
+ ADD_PADDING(st, pad, fs - len);
}
- ADD_STRN(str, len)
+ ADD_STRN(st, str, len);
}
}
} /* add_aligned() */
@@ -1403,7 +1404,7 @@
for (; i < TAB->nocols; i++)
{
/* TAB->size is not negative. */
- ADD_CHARN(' ', done)
+ ADD_CHARN(st, ' ', done);
}
}
@@ -1661,11 +1662,11 @@
*/
if (column_stat == 2)
- ADD_CHAR('\n');
+ ADD_CHAR(st, '\n');
column_stat = 0;
if (!format_str[fpos])
break;
- ADD_CHAR('\n');
+ ADD_CHAR(st, '\n');
st->line_start = st->bpos;
continue;
}
@@ -1673,7 +1674,7 @@
column_stat = 0; /* If there was a newline pending, it
* will be implicitely added now.
*/
- ADD_CHAR('\n');
+ ADD_CHAR(st, '\n');
st->line_start = st->bpos;
/* Handle pending columns and tables */
@@ -1692,7 +1693,7 @@
while (*((*temp)->d.col) == ' ')
(*temp)->d.col++;
i = (*temp)->start - (st->bpos - st->line_start);
- ADD_CHARN(' ', i);
+ ADD_CHARN(st, ' ', i);
column_stat = add_column(st, temp);
if (!column_stat)
temp = &((*temp)->next);
@@ -1701,19 +1702,19 @@
{
i = (*temp)->start - (st->bpos - st->line_start);
if (i > 0)
- ADD_CHARN(' ', i);
+ ADD_CHARN(st, ' ', i);
if (!add_table(st, temp))
temp = &((*temp)->next);
}
} /* while (*temp) */
if (st->csts || format_str[fpos] == '\n')
- ADD_CHAR('\n');
+ ADD_CHAR(st, '\n');
st->line_start = st->bpos;
} /* while (csts) */
if (column_stat == 2 && format_str[fpos] != '\n')
- ADD_CHAR('\n');
+ ADD_CHAR(st, '\n');
if (!format_str[fpos])
break;
@@ -1726,16 +1727,16 @@
if (format_str[fpos+1] == '%')
{
- ADD_CHAR('%');
+ ADD_CHAR(st, '%');
fpos++;
continue;
}
if (format_str[fpos+1] == '^')
{
- ADD_CHAR('%');
+ ADD_CHAR(st, '%');
fpos++;
- ADD_CHAR('^');
+ ADD_CHAR(st, '^');
continue;
}
@@ -1935,7 +1936,7 @@
/* never reached... */
fprintf(stderr, "%s: (s)printf: INFO_T_NULL.... found.\n"
, get_txt(current_object->name));
- ADD_CHAR('%');
+ ADD_CHAR(st, '%');
break;
}
@@ -2192,7 +2193,7 @@
}
else
{
- ADD_STRN(get_txt(carg->u.str), slen)
+ ADD_STRN(st, get_txt(carg->u.str), slen);
}
}
break;
@@ -2238,7 +2239,7 @@
if ((unsigned int)tmpl < fs)
add_aligned(st, temp, tmpl, pad, fs, finfo);
else
- ADD_STRN(temp, tmpl)
+ ADD_STRN(st, temp, tmpl);
break;
}
else
@@ -2344,14 +2345,14 @@
* with leading zeroes: preserve the sign
* character in the right place.
*/
- ADD_STRN(temp, 1);
+ ADD_STRN(st, temp, 1);
add_aligned(st, temp+1, tmpl-1, pad, fs-1, finfo);
}
else
add_aligned(st, temp, tmpl, pad, fs, finfo);
}
else
- ADD_STRN(temp, tmpl)
+ ADD_STRN(st, temp, tmpl);
break;
}
default: /* type not found */
@@ -2371,10 +2372,10 @@
} /* if format entry */
/* Nothing to format: just copy the character */
- ADD_CHAR(format_str[fpos]);
+ ADD_CHAR(st, format_str[fpos]);
} /* for (fpos=0; 1; fpos++) */
- ADD_CHAR('\0'); /* Terminate the formatted string */
+ ADD_CHAR(st, '\0'); /* Terminate the formatted string */
/* Restore characters */
while (st->saves)
| ||||
|
|
Ah, I just had a look at the linked talk again. FTR: it actually recommends to use only "static" because the compiler will then inline small functions or functions which are called only once anyway. That is probably even true... |
|
|
d) expression safety: BLUBB(foo++) #define BLUBB(x) ((x) + (x)) vs. inline int BLUBB(int x) { return x+x; } |
|
|
Uh, thank you Largo. I didn't even notice that you attached patches here (probably best to attach a note as well). I will have a look during the next days. |
|
|
The patches seem to be fine for me with one small change: inline -> static INLINE to prevent gcc from emitting a stand-alone copy of the function, if it sucessfually inlines it everywhere. (INLINE as Makro for now, until we eventually require (most of) C99 in 3.5.x) Actually, 'static' alone may be fully sufficient because the compiler usually decides itself if it honors the request for inlining or not. But I guess, it does not harm either as kind of hint for the programmer. There is another possibility: to declare the function as 'extern inline' in all but one compilation unit, but it is a little more complicated to do so and I am not sure if it is worth the effort. |
|
|
The #defines in bytecode.h are also a good candidate for this, especially in the light that we have to check the types very carefully on LP64 platforms. If we use typed functions the compiler might help us (like in 0000559). Unfortunately, some of them increment/decrement an argument: #define STORE_CODE(p,c) (*(p)++ = (c)) A possibility is to use a pointer to pointer as argument: static INLINE bytecode_t store_code (bytecode_p *p, bytecode_t c) { *((*p)++) = c; return c; } together with a #define STORE_CODE(p,c) store_code (&(p),(c)) Or we can get rid of the macro altogether with something like: %s/STORE_CODE(\(.*\), \(.*\))/store_code\(\&\(\1\), \(\2\)\)/gc in vim which I would actually prefer. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2008-09-09 14:16 | zesstra | New Issue | |
| 2008-09-09 14:16 | zesstra | Status | new => assigned |
| 2008-09-09 14:16 | zesstra | Assigned To | => zesstra |
| 2008-09-09 14:50 | zesstra | Note Added: 0000768 | |
| 2008-09-09 15:00 | Largo | Note Added: 0000769 | |
| 2008-09-10 14:15 | Largo | File Added: comm.c.diff | |
| 2008-09-10 15:39 | Largo | File Added: alloc.diff | |
| 2008-09-11 13:50 | Largo | File Added: sprintf.c.diff | |
| 2008-12-23 07:19 | zesstra | Note Added: 0000826 | |
| 2008-12-29 05:09 | zesstra | Note Added: 0000835 | |
| 2008-12-29 05:15 | zesstra | Additional Information Updated | |
| 2009-01-04 17:10 | zesstra | Relationship added | related to 0000559 |
| 2009-01-06 07:23 | zesstra | Note Added: 0000853 | |
| 2009-02-08 09:12 | zesstra | Relationship added | parent of 0000606 |
