[Asterisk-Dev] [RFC] strncpy -> ast_copy_string

[Luigi Rizzo]

sorry to jump in the middle of the discussion without having
looked at the beginning, but just managed to subscribe to the
list and have only looked at asterisk sources for a couple of weeks.

I notice that a lot of code in asterisk uses statically sized buffers,
which is inefficient memorywise, risky if you forget or
misspecify the 'size' in the various stncpy, and might lead
to unexpected behaviour because of arbitrary truncation of the strings.

I think that over time one should switch to support variable-size
strings, and define an appropriate set of macros or functions
to operate on them.
Certainly deprecating strncpy in favour of a private method is
a first step.

One area i have been looking at is the parsing of config files,
where the typical sequence is

	if (!strcasecmp((v->name), "foo")) { f1; } else
	if (!strcasecmp((v->name), "bar")) { f2; } else
	if (!strcasecmp((v->name), "bar")) { f3; } else
	...

where f1 f2 f3 are chunks of code where you copy the value to a string,
int, boolean or something similar.

The following set of macros gives some advantages in readability
to the sections of code in charge of parsing config files or
similar textual fields (the M_ in the macro stands for "match")

#define M_START(var, val) \
        char *__s = var; char *__val = val;
#define M_END(x)   x;
#define M_F(tag, f)             if (!strcasecmp((__s), tag)) { f; } else
#define M_BOOL(tag, dst)        M_F(tag, (dst) = ast_true(__val) )
#define M_UINT(tag, dst)        M_F(tag, (dst) = strtoul(__val, NULL, 0) )
#define M_STR(tag, dst)         M_F(tag, strncpy(dst, __val, sizeof(dst) - 1) )

examples of use from chan_sip.c:

	if (rowval[x]) {
		M_START(fields[x].name, rowval[x]);

		M_STR("secret", p->secret)
		M_STR("name", p->name)
		M_STR("context", p->context)
		M_STR("username", p->username)
		M_F("ipaddr",
			inet_aton(rowval[x], &p->addr.sin_addr);)
		M_F("port",
			if (sscanf(rowval[x], "%i", &port) != 1)
				port = 0;
				p->addr.sin_port = htons(port);)
		M_F("regseconds",
			if (sscanf(rowval[x], "%li", &regseconds) != 1)
				regseconds = 0;) /* XXX maybe M_UINT ? */
		M_END(;)
	}

and later below:

	while(v) {
		M_START(v->name, v->value);

		M_STR("context", user->context)
		M_F("permit",
			user->ha = ast_append_ha(v->name, v->value, user->ha);)
		M_F("deny",
			user->ha = ast_append_ha(v->name, v->value, user->ha);)
		M_STR("secret", user->secret)
		M_STR("md5secret", user->md5secret)  
		M_BOOL("promiscredir", user->promiscredir)
		M_F("dtmfmode",
			if (!strcasecmp(v->value, "inband"))
				user->dtmfmode=SIP_DTMF_INBAND;
			else if (!strcasecmp(v->value, "rfc2833"))
				user->dtmfmode = SIP_DTMF_RFC2833;
			else if (!strcasecmp(v->value, "info"))
				user->dtmfmode = SIP_DTMF_INFO;
			else {
				ast_log(LOG_WARNING,
				    "Unknown dtmf mode '%s', using rfc2833\n",
				    v->value);
				user->dtmfmode = SIP_DTMF_RFC2833;
			})
		...
		M_END(;)  
		v = v->next;
	}

note how all the abive is a lot more readable than the original code involving
a lot of strncpy() stuff, especially if we provide other macros for
parsing standard types such as ports and IP addresses, times and things
like "dtmfmode" which are replicated many many times in the code.

This type of code (without helper macros or functions) is very error prone
to write, and it is unlikely that trivial
tests can catch errors in the copy&paste process.

The use of masking macros will also help a lot in the transition to different
string management procedures.

	cheers
	luigi

On Mon, May 02, 2005 at 07:45:14AM -0700, Ledion B wrote:
> Hey Kevin,
> 
> Can we see the code for your test_copy_string and
> test_strncpy.
> 
> >From my understanding of the documentation of strncpy,
> the padding will only happen if n > strlen(src), which
> makes sense. If you make sure than n !> strlen(src),
> then no padding happens.
> 
> Here is a link to the strncpy documentation
> http://www.opengroup.org/onlinepubs/007908799/xsh/strncpy.html
> 
> __________________________________________________
> Do You Yahoo!?
> Tired of spam?  Yahoo! Mail has the best spam protection around 
> http://mail.yahoo.com 
> _______________________________________________
> Asterisk-Dev mailing list
> Asterisk-Dev at lists.digium.com
> http://lists.digium.com/mailman/listinfo/asterisk-dev
> To UNSUBSCRIBE or update options visit:
>    http://lists.digium.com/mailman/listinfo/asterisk-dev