On 12-04-24 8:01 AM, Kenneth R Westerback wrote:
On Tue, Apr 24, 2012 at 01:05:52AM -0400, Mansour Moufid wrote:
In getenv.c, the __findenv function is changed to use size_t for its
length parameter, and ptrdiff_t for its offset parameter.
The getenv function is modified accordingly.
In setenv.c, the setenv function is also modified to match the changes
to __findenv, as well as to use size_t for all variables representing
object sizes.
---
Even better than in-lining the diff would be to provide a cvs diff
rather than a git diff. The official tree is distributed and
maintained in cvs, and thus we can't know what or where your git
tree comes from. A cvs diff also usually includes the information
necessary to find out where the diff applies. And some developers just
don't react well to git. :-)
Oops, sorry. I appended a cvs diff at the end of this message.
Just reading the diff, since I'm not sure how or where to apply it,
I can't see much need for or concern about environment information
larger than MAXINT in size. Nor am I a fan of the (to me) silly
proliferation of types like ptrdiff_t.
Good point. Here, because of the for loops, the difference is
always positive, so I changed it to size_t.
Is there some standards/known constraint/current behaviour on other
unix's that this would address?
Nothing in particular. But using signed types to represent lengths
is generally considered harmful, especially in tandem with malloc.
Also, the behaviour of a cast to int (signed) from size_t
(unsigned; as with l_value below) is implementation-defined when
the value is not representable in the signed type. Other than
that, this patch is just for the sake of correctness.
Thanks for the feedback. I'm mostly curious if the OpenBSD
developer community is interested in typing issues. I'm working on
a static analysis tool (+ source transformation; this is the type
of output it produces), and testing the waters.
Mansour
Index: stdlib/getenv.c
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/getenv.c,v
retrieving revision 1.10
diff -u -r1.10 getenv.c
--- stdlib/getenv.c 23 Aug 2010 22:31:50 -0000 1.10
+++ stdlib/getenv.c 24 Apr 2012 19:33:10 -0000
@@ -31,7 +31,7 @@
#include <stdlib.h>
#include <string.h>
-char *__findenv(const char *name, int len, int *offset);
+char *__findenv(const char *name, size_t len, size_t *offset);
/*
* __findenv --
@@ -44,10 +44,10 @@
* This routine *should* be a static; don't use it.
*/
char *
-__findenv(const char *name, int len, int *offset)
+__findenv(const char *name, size_t len, size_t *offset)
{
extern char **environ;
- int i;
+ size_t i;
const char *np;
char **p, *cp;
@@ -72,10 +72,10 @@
char *
getenv(const char *name)
{
- int offset = 0;
+ size_t offset = 0;
const char *np;
for (np = name; *np && *np != '='; ++np)
;
- return (__findenv(name, (int)(np - name), &offset));
+ return (__findenv(name, (size_t)(np - name), &offset));
}
Index: stdlib/setenv.c
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/setenv.c,v
retrieving revision 1.13
diff -u -r1.13 setenv.c
--- stdlib/setenv.c 23 Aug 2010 22:31:50 -0000 1.13
+++ stdlib/setenv.c 24 Apr 2012 19:33:10 -0000
@@ -32,7 +32,7 @@
#include <stdlib.h>
#include <string.h>
-char *__findenv(const char *name, int len, int *offset);
+char *__findenv(const char *name, size_t len, size_t *offset);
extern char **environ;
static char **lastenv; /* last value of environ */
@@ -47,7 +47,7 @@
{
char **P, *cp;
size_t cnt;
- int offset = 0;
+ size_t offset = 0;
for (cp = str; *cp && *cp != '='; ++cp)
;
@@ -56,10 +56,10 @@
return (-1); /* missing `=' in string */
}
- if (__findenv(str, (int)(cp - str), &offset) != NULL) {
+ if (__findenv(str, (size_t)(cp - str), &offset) != NULL) {
environ[offset++] = str;
/* could be set multiple times */
- while (__findenv(str, (int)(cp - str), &offset)) {
+ while (__findenv(str, (size_t)(cp - str), &offset)) {
for (P = &environ[offset];; ++P)
if (!(*P = *(P + 1)))
break;
@@ -92,7 +92,7 @@
{
char *C, **P;
const char *np;
- int l_value, offset = 0;
+ size_t value_len, name_len, offset = 0;
for (np = name; *np && *np != '='; ++np)
;
@@ -102,21 +102,22 @@
return (-1); /* has `=' in name */
}
#endif
+ name_len = (size_t)(np - name);
- l_value = strlen(value);
- if ((C = __findenv(name, (int)(np - name), &offset)) != NULL) {
- int tmpoff = offset + 1;
+ value_len = strlen(value);
+ if ((C = __findenv(name, name_len, &offset)) != NULL) {
+ size_t tmpoff = offset + 1;
if (!rewrite)
return (0);
#if 0 /* XXX - existing entry may not be writable */
- if (strlen(C) >= l_value) { /* old larger; copy over */
+ if (strlen(C) >= value_len) { /* old larger; copy over */
while ((*C++ = *value++))
;
return (0);
}
#endif
/* could be set multiple times */
- while (__findenv(name, (int)(np - name), &tmpoff)) {
+ while (__findenv(name, name_len, &tmpoff)) {
for (P = &environ[tmpoff];; ++P)
if (!(*P = *(P + 1)))
break;
@@ -136,8 +137,8 @@
offset = cnt;
environ[cnt + 1] = NULL;
}
- if (!(environ[offset] = /* name + `=' + value */
- malloc((size_t)((int)(np - name) + l_value + 2))))
+ /* name + `=' + value */
+ if (!(environ[offset] = malloc(name_len + value_len + 2)))
return (-1);
for (C = environ[offset]; (*C = *name++) && *C != '='; ++C)
;
@@ -155,7 +156,7 @@
{
char **P;
const char *np;
- int offset = 0;
+ size_t offset = 0;
if (!name || !*name) {
errno = EINVAL;
@@ -169,7 +170,7 @@
}
/* could be set multiple times */
- while (__findenv(name, (int)(np - name), &offset)) {
+ while (__findenv(name, (size_t)(np - name), &offset)) {
for (P = &environ[offset];; ++P)
if (!(*P = *(P + 1)))
break;
