minor fixes

minor fixes

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]

attached a few lines of small changes


* if the time.h fixme is because of the sigevent struct
declaration, then it can go away as posix says
"The tag sigevent shall be declared as naming an incomplete structure type,
the contents of which are described in the <signal.h> header."

actually there is a much stronger statement as well:
"Inclusion of the <time.h> header may make visible all symbols from the
<signal.h> header."


* __syscall_ret: declaration should be consistent with the function
definition (argument is signed long)

i think one of the (unsigned long) casts is unnecessary due to conversion
rules but it probably makes things more clear


* sbrk: changed prototype here as well to be consistent with the standard

actually semantically ptrdiff_t is better (sbrk does p+arg, not p=arg)
but the standard uses intptr_t

this change should not hurt as sbrk should not be used anyway..
(i guess there is no architecture where intptr_t != ptrdiff_t)


* __asctime: use new crash (i assume the *0=0 crash is deprecated now)


* setpgid: pid_t return type is wrong (return value is an error code)


[-- Attachment #2: minor.diff --]
[-- Type: text/x-diff, Size: 2149 bytes --]

diff --git a/include/time.h b/include/time.h
index 5b1ea91..557c8f4 100644
--- a/include/time.h
+++ b/include/time.h
@@ -85,7 +85,6 @@ int clock_settime (clockid_t, const struct timespec *);
 int clock_nanosleep (clockid_t, int, const struct timespec *, struct timespec *);
 int clock_getcpuclockid (pid_t, clockid_t *);
 
-/* FIXME..?? */
 struct sigevent;
 int timer_create (clockid_t, struct sigevent *, timer_t *);
 int timer_delete (timer_t);
diff --git a/src/internal/syscall_ret.c b/src/internal/syscall_ret.c
index 4f159e0..4949a9c 100644
--- a/src/internal/syscall_ret.c
+++ b/src/internal/syscall_ret.c
@@ -1,11 +1,11 @@
 #include <errno.h>
 #include <unistd.h>
 
-long __syscall_ret(unsigned long r)
+long __syscall_ret(long r)
 {
-	if (r >= (unsigned long)-1 - 4096) {
-		errno = -(long)r;
+	if ((unsigned long)r >= (unsigned long)-1 - 4096) {
+		errno = -r;
 		return -1;
 	}
-	return (long)r;
+	return r;
 }
diff --git a/src/linux/sbrk.c b/src/linux/sbrk.c
index 5fab74b..3643765 100644
--- a/src/linux/sbrk.c
+++ b/src/linux/sbrk.c
@@ -1,7 +1,7 @@
-#include <stddef.h>
+#include <stdint.h>
 #include "syscall.h"
 
-void *sbrk(ptrdiff_t inc)
+void *sbrk(intptr_t inc)
 {
 	unsigned long cur = syscall(SYS_brk, 0);
 	if (inc && syscall(SYS_brk, cur+inc) != cur+inc) return (void *)-1;
diff --git a/src/time/__asctime.c b/src/time/__asctime.c
index d31f634..7cc4f50 100644
--- a/src/time/__asctime.c
+++ b/src/time/__asctime.c
@@ -1,6 +1,7 @@
 #include <time.h>
 #include <stdio.h>
 #include <langinfo.h>
+#include "atomic.h"
 
 const char *__langinfo(nl_item);
 
@@ -21,7 +22,7 @@ char *__asctime(const struct tm *tm, char *buf)
 		 * application developers that they may not be so lucky
 		 * on other implementations (e.g. stack smashing..).
 		 */
-		*(volatile int*)0 = 0;
+		a_crash();
 	}
 	return buf;
 }
diff --git a/src/unistd/setpgid.c b/src/unistd/setpgid.c
index 4a5a3d6..0616069 100644
--- a/src/unistd/setpgid.c
+++ b/src/unistd/setpgid.c
@@ -1,7 +1,7 @@
 #include <unistd.h>
 #include "syscall.h"
 
-pid_t setpgid(pid_t pid, pid_t pgid)
+int setpgid(pid_t pid, pid_t pgid)
 {
 	return syscall(SYS_setpgid, pid, pgid);
 }

Re: minor fixes

[Rich Felker]

On Sun, Sep 25, 2011 at 03:47:25PM +0200, Szabolcs Nagy wrote:
> attached a few lines of small changes
> 
> 
> * if the time.h fixme is because of the sigevent struct
> declaration, then it can go away as posix says
> "The tag sigevent shall be declared as naming an incomplete structure type,
> the contents of which are described in the <signal.h> header."
> 
> actually there is a much stronger statement as well:
> "Inclusion of the <time.h> header may make visible all symbols from the
> <signal.h> header."

OK, I guess this is what the comment was about. Actually I forgot what
it was there for. :-)

> * __syscall_ret: declaration should be consistent with the function
> definition (argument is signed long)
> 
> i think one of the (unsigned long) casts is unnecessary due to conversion
> rules but it probably makes things more clear

I want to keep the types from the function definition and fix the
prototype. They were basically chosen so that the "ugly"
(impl-defined) conversion (unsigned-to-signed) would always take place
inside the implementation of this function, and the safe
(signed-to-unsigned) conversion could take place implicitly in the
caller if necessary. It probably doesn't matter but I thought it
cleaner at the time.

> * sbrk: changed prototype here as well to be consistent with the standard
> 
> actually semantically ptrdiff_t is better (sbrk does p+arg, not p=arg)
> but the standard uses intptr_t
> 
> this change should not hurt as sbrk should not be used anyway..
> (i guess there is no architecture where intptr_t != ptrdiff_t)

I agree, this is the right fix.

> * __asctime: use new crash (i assume the *0=0 crash is deprecated now)

Good catch.

> * setpgid: pid_t return type is wrong (return value is an error code)

This one too.

Thanks!

Rich

Re: minor fixes

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 266 bytes --]

a pedantic ctype macro fix: move the unsigned cast inside

i can think of one case where it matters:
if EOF was defined as INT_MIN then
(unsigned)(a - 'c') can overflow while ((unsigned)a - 'c') is ok

(i know this is pathological and EOF is actually defined as -1)

[-- Attachment #2: ctype_cast.diff --]
[-- Type: text/x-diff, Size: 798 bytes --]

diff --git a/include/ctype.h b/include/ctype.h
index 97b9737..a605d08 100644
--- a/include/ctype.h
+++ b/include/ctype.h
@@ -16,12 +16,12 @@ int   isxdigit(int);
 int   tolower(int);
 int   toupper(int);
 
-#define isalpha(a) ((unsigned)(((a)|32)-'a') < 26)
-#define isdigit(a) ((unsigned)((a)-'0') < 10)
-#define islower(a) ((unsigned)((a)-'a') < 26)
-#define isupper(a) ((unsigned)((a)-'A') < 26)
-#define isprint(a) ((unsigned)((a)-0x20) < 0x5f)
-#define isgraph(a) ((unsigned)((a)-0x21) < 0x5e)
+#define isalpha(a) ((((unsigned)(a)|32)-'a') < 26)
+#define isdigit(a) (((unsigned)(a)-'0') < 10)
+#define islower(a) (((unsigned)(a)-'a') < 26)
+#define isupper(a) (((unsigned)(a)-'A') < 26)
+#define isprint(a) (((unsigned)(a)-0x20) < 0x5f)
+#define isgraph(a) (((unsigned)(a)-0x21) < 0x5e)