[PATCH] do not check errno unless a system call fails, fix !SOCK_CLOEXEC case

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] do not check errno unless a system call fails, fix !SOCK_CLOEXEC case

Paolo Bonzini-2
2010-03-27  Paolo Bonzini <[hidden email]>

        * kernel/File.st: Do not check errno unless a system call fails.
        * kernel/Directory.st: Likewise.
        * kernel/FileDescr.st: Likewise.  Remove useless #...ifFail: keywords.

packages/sockets:
2010-03-27  Paolo Bonzini <[hidden email]>

        * AbstractSocketImpl.st: Do not check errno unless a system call
        fails.

libgst:
2010-03-25  Paolo Bonzini  <[hidden email]>

        * libgst/sockets.c: Fix logic for no SOCK_CLOEXEC or no accept4.
---
 ChangeLog                              |    6 +++
 kernel/Directory.st                    |   10 +++---
 kernel/File.st                         |   63 ++++++++++++++++----------------
 kernel/FileDescr.st                    |    7 +---
 libgst/ChangeLog                       |    4 ++
 libgst/sockets.c                       |    4 ++-
 packages/sockets/AbstractSocketImpl.st |   13 +++----
 packages/sockets/ChangeLog             |    5 +++
 8 files changed, 63 insertions(+), 49 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3141f05..f549e5b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2010-03-27  Paolo Bonzini <[hidden email]>
+
+ * kernel/File.st: Do not check errno unless a system call fails.
+ * kernel/Directory.st: Likewise.
+ * kernel/FileDescr.st: Likewise.  Remove useless #...ifFail: keywords.
+
 2010-03-25  Holger Hans Peter Freyther  <[hidden email]>
 
  * kernel/Integer.st: Refer to the right parameter of the selectors.
diff --git a/kernel/Directory.st b/kernel/Directory.st
index bb1eab8..1a67caa 100644
--- a/kernel/Directory.st
+++ b/kernel/Directory.st
@@ -51,7 +51,7 @@ how to use the instances returned by my class methods.'>
 
     Directory class >> primWorking: dirName [
  <category: 'private-C call-outs'>
- <cCall: 'chdir' returning: #void args: #(#string)>
+ <cCall: 'chdir' returning: #int args: #(#string)>
 
     ]
 
@@ -204,8 +204,8 @@ how to use the instances returned by my class methods.'>
  "Change the current working directory to dirName."
 
  <category: 'file operations'>
- self primWorking: dirName asString.
- File checkError
+ (self primWorking: dirName asString) < 0
+            ifTrue: [File checkError].
     ]
 
     Directory class >> createTemporary: prefix [
@@ -214,8 +214,8 @@ how to use the instances returned by my class methods.'>
  <category: 'file operations'>
  | name |
  name := prefix asString , 'XXXXXX'.
- self primCreateTemporary: name.
- File checkError.
+ (self primCreateTemporary: name) isNil
+            ifTrue: [File checkError].
  ^File name: name
     ]
 
diff --git a/kernel/File.st b/kernel/File.st
index 6aa4de7..ab6db58 100644
--- a/kernel/File.st
+++ b/kernel/File.st
@@ -262,31 +262,31 @@ FilePath subclass: File [
 
     primSymlink: srcName as: destName [
  <category: 'private-C call-outs'>
- <cCall: 'symlink' returning: #void args: #(#string #string)>
+ <cCall: 'symlink' returning: #int args: #(#string #string)>
 
     ]
 
     primUnlink: fileName [
  <category: 'private-C call-outs'>
- <cCall: 'unlink' returning: #void args: #(#string)>
+ <cCall: 'unlink' returning: #int args: #(#string)>
 
     ]
 
     primRename: oldFileName to: newFileName [
  <category: 'private-C call-outs'>
- <cCall: 'rename' returning: #void args: #(#string #string)>
+ <cCall: 'rename' returning: #int args: #(#string #string)>
 
     ]
 
     primRemoveDir: fileName [
  <category: 'private-C call-outs'>
- <cCall: 'rmdir' returning: #void args: #(#string)>
+ <cCall: 'rmdir' returning: #int args: #(#string)>
 
     ]
 
     primCreateDir: dirName mode: mode [
  <category: 'private-C call-outs'>
- <cCall: 'mkdir' returning: #void args: #(#string #int)>
+ <cCall: 'mkdir' returning: #int args: #(#string #int)>
 
     ]
 
@@ -348,8 +348,8 @@ FilePath subclass: File [
  anInteger."
 
  <category: 'accessing'>
- self primChmod: self asString mode: (anInteger bitAnd: 4095).
- File checkError
+ (self primChmod: self asString mode: (anInteger bitAnd: 4095)) < 0
+    ifTrue: [ File checkError ]
     ]
 
     isFileSystemPath [
@@ -423,8 +423,8 @@ FilePath subclass: File [
 
  <category: 'accessing'>
  stat isNil ifTrue: [stat := Kernel.Stat new].
- self lstatOn: self asString into: stat.
- File checkError.
+ (self lstatOn: self asString into: stat) < 0
+            ifTrue: [File checkError].
  isSymbolicLink := (stat stMode bitAnd: 61440) = 40960. "S_IFLNK"
  isSymbolicLink
     ifTrue:
@@ -437,8 +437,8 @@ FilePath subclass: File [
 
  <category: 'testing'>
  stat isNil ifTrue: [stat := Kernel.Stat new].
- self lstatOn: self asString into: stat.
- File errno == 0 ifFalse: [^false].
+ (self lstatOn: self asString into: stat) < 0
+    ifTrue: [^false].
  isSymbolicLink := (stat stMode bitAnd: 61440) = 40960. "S_IFLNK"
  isSymbolicLink ifTrue: [self statOn: self asString into: stat].
  ^true
@@ -487,22 +487,22 @@ FilePath subclass: File [
  "Set the receiver's owner and group to be ownerString and groupString."
 
  <category: 'file operations'>
- self class
+ (self class
     setOwnerFor: self asString
     owner: ownerString
-    group: groupString.
- File checkError
+    group: groupString) < 0
+        ifTrue: [ File checkError ]
     ]
 
     lastAccessTime: accessDateTime lastModifyTime: modifyDateTime [
  "Set the receiver's timestamps to be accessDateTime and modifyDateTime."
 
  <category: 'file operations'>
- self class
+ (self class
     setTimeFor: self asString
     atime: (self secondsFromDateTime: accessDateTime)
-    mtime: (self secondsFromDateTime: modifyDateTime).
- File checkError
+    mtime: (self secondsFromDateTime: modifyDateTime)) < 0
+        ifTrue: [ File checkError ]
     ]
 
     open: class mode: mode ifFail: aBlock [
@@ -520,26 +520,27 @@ FilePath subclass: File [
  "Remove the file with the given path name"
 
  <category: 'file operations'>
- self isDirectory
+        | result |
+ result := self isDirectory
     ifTrue: [self primRemoveDir: self asString]
     ifFalse: [self primUnlink: self asString].
- File checkError
+ result < 0 ifTrue: [ File checkError ]
     ]
 
     symlinkFrom: srcName [
  "Create the receiver as a symlink from path destName"
 
  <category: 'file operations'>
- self primSymlink: srcName as: self asString.
- File checkError
+ (self primSymlink: srcName as: self asString) < 0
+            ifTrue: [ File checkError ]
     ]
 
     renameTo: newFileName [
  "Rename the file with the given path name to newFileName"
 
  <category: 'file operations'>
- self primRename: self asString to: newFileName.
- File checkError
+ (self primRename: self asString to: newFileName) < 0
+    ifTrue: [ File checkError ]
     ]
 
     secondsFromDateTime: aDateTime [
@@ -572,8 +573,8 @@ FilePath subclass: File [
  "Create the receiver as a directory."
 
  <category: 'directory operations'>
- self primCreateDir: self asString mode: 511.
- File checkError
+ (self primCreateDir: self asString mode: 8r777) < 0
+    ifTrue: [ File checkError ]
     ]
 
     namesDo: aBlock [
@@ -583,13 +584,13 @@ FilePath subclass: File [
  <category: 'directory operations'>
  | dir entry |
  dir := self openDir: self asString.
- File checkError.
+ dir isNil ifTrue: [^File checkError].
 
- [[entry := self readDir: dir.
-    File checkError.
-    entry notNil]
- whileTrue: [aBlock value: (self extractDirentName: entry)]]
-    ensure: [self closeDir: dir]
+ [[(entry := self readDir: dir) notNil]
+    whileTrue: [aBlock value: (self extractDirentName: entry)]]
+    ensure: [self closeDir: dir].
+
+ File checkError.
     ]
 
     symlinkAs: destName [
diff --git a/kernel/FileDescr.st b/kernel/FileDescr.st
index 825fc96..8f1d23f 100644
--- a/kernel/FileDescr.st
+++ b/kernel/FileDescr.st
@@ -304,7 +304,6 @@ do arbitrary processing on the files.'>
  self isOpen ifFalse: [^self].
  self flush.
  self fileOp: 19.
- self checkError.
  access := 1
     ]
 
@@ -799,8 +798,7 @@ do arbitrary processing on the files.'>
     fileOp: 3
     with: aCollection
     with: position + available
-    with: (position + n - 1 min: aCollection size)
-    ifFail: [self checkError].
+    with: (position + n - 1 min: aCollection size).
  count := count + available.
  count = 0 ifTrue: [atEnd := true].
  ^count
@@ -820,8 +818,7 @@ do arbitrary processing on the files.'>
     fileOp: 2
     with: aCollection
     with: cur
-    with: last
-    ifFail: [self checkError].
+    with: last.
  result = 0 ifTrue: [^cur - position].
  cur := cur + result].
  ^cur - position
diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index 3c30589..d28f24c 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,3 +1,7 @@
+2010-03-25  Paolo Bonzini  <[hidden email]>
+
+ * libgst/sockets.c: Fix logic for no SOCK_CLOEXEC or no accept4.
+
 2010-02-20  Paolo Bonzini  <[hidden email]>
 
  * libgst/files.c: Adjust for AnsiExcept.st rename.
diff --git a/libgst/sockets.c b/libgst/sockets.c
index 3e155d6..f8e8a63 100644
--- a/libgst/sockets.c
+++ b/libgst/sockets.c
@@ -236,12 +236,14 @@ constantFunction (aiV4mapped, AI_V4MAPPED)
 /* 0 = unknown, 1 = yes, -1 = no.  */
 static mst_Boolean have_sock_cloexec;
 
+/* Return 0 if the operation failed and an error can be returned
+   by the caller.  */
 static inline int
 check_have_sock_cloexec (int fh, int expected_errno)
 {
   if (have_sock_cloexec == 0 && (fh >= 0 || errno == expected_errno))
     have_sock_cloexec = (fh >= 0 ? 1 : -1);
-  return (have_sock_cloexec == 1);
+  return (have_sock_cloexec != 0);
 }
 #endif
 
diff --git a/packages/sockets/AbstractSocketImpl.st b/packages/sockets/AbstractSocketImpl.st
index 961b5a7..a71410e 100644
--- a/packages/sockets/AbstractSocketImpl.st
+++ b/packages/sockets/AbstractSocketImpl.st
@@ -75,7 +75,7 @@ this can be changed by class methods on SocketAddress sublcasses.'>
     create: addressClass protocolFamily
     type: self socketType
     protocol: self protocol.
- File checkError.
+ descriptor < 0 ifTrue: [ File checkError ].
  ^self on: descriptor
     ]
 
@@ -93,6 +93,7 @@ this can be changed by class methods on SocketAddress sublcasses.'>
     accept: fd
     peer: peer
     addrLen: addrLen.
+        newFD < 0 ifTrue: [ File checkError: self soError ].
  ^(implementationClass on: newFD)
     hasBeenBound;
     hasBeenConnectedTo: peer;
@@ -108,11 +109,10 @@ this can be changed by class methods on SocketAddress sublcasses.'>
  addr := ipAddress port: port.
 
  (fd := self fd) isNil ifTrue: [ ^self ].
- [self
+ [(self
     bind: fd
     to: addr
-    addrLen: addr size.
- File checkError]
+    addrLen: addr size) < 0 ifTrue: [File checkError: self soError] ]
  ifCurtailed: [self close].
  self isOpen ifTrue: [self hasBeenBound]
     ]
@@ -527,11 +527,10 @@ implementations.'>
  addr := ipAddress port: port.
 
  [(fd := self fd) isNil ifTrue: [ ^self ].
- self
+ (self
     connect: fd
     to: addr
-    addrLen: addr size.
- File checkError]
+    addrLen: addr size) < 0 ifTrue: [File checkError: self soError] ]
  ifCurtailed: [self close].
 
  "connect does not block, so wait for"
diff --git a/packages/sockets/ChangeLog b/packages/sockets/ChangeLog
index ab405c7..4ed3aba 100644
--- a/packages/sockets/ChangeLog
+++ b/packages/sockets/ChangeLog
@@ -1,3 +1,8 @@
+2010-03-27  Paolo Bonzini <[hidden email]>
+
+ * AbstractSocketImpl.st: Do not check errno unless a system call
+ fails.
+
 2010-01-01  Paolo Bonzini  <[hidden email]>
 
  * Update copyright years.
--
1.6.6.1



_______________________________________________
help-smalltalk mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/help-smalltalk