diff mbox

ctermid: return string literal, document MT-Safety pitfall

Message ID or8ujlsria.fsf@free.home
State New
Headers show

Commit Message

Alexandre Oliva Nov. 8, 2014, 2:20 p.m. UTC 
On Nov  7, 2014, Richard Henderson <rth@twiddle.net> wrote:

> On 11/07/2014 09:35 AM, Alexandre Oliva wrote:
>> char *
>> ctermid (s)
>> char *s;

> Can you please fix the K&R at the same time?

Sure, how's this?


ctermid: return string literal, document MT-Safety pitfall

From: Alexandre Oliva <aoliva@redhat.com>

for  ChangeLog

	* sysdeps/posix/ctermid.c (ctermid): Return a pointer to a
	string literal if not passed a buffer.
	* manual/job.texi (ctermid): Update reasoning, note deviation
	from posix, suggest mtasurace when not passed a buffer, for
	future non-preliminary safety notes.
---
 manual/job.texi         |    8 +++++---
 sysdeps/posix/ctermid.c |   16 ++++++++--------
 2 files changed, 13 insertions(+), 11 deletions(-)

Comments

Richard Henderson Nov. 8, 2014, 3:01 p.m. UTC | #1 
On 11/08/2014 03:20 PM, Alexandre Oliva wrote:
> ctermid: return string literal, document MT-Safety pitfall
> 
> From: Alexandre Oliva <aoliva@redhat.com>
> 
> for  ChangeLog
> 
> 	* sysdeps/posix/ctermid.c (ctermid): Return a pointer to a
> 	string literal if not passed a buffer.
> 	* manual/job.texi (ctermid): Update reasoning, note deviation
> 	from posix, suggest mtasurace when not passed a buffer, for
> 	future non-preliminary safety notes.

LGTM.


r~
Torvald Riegel Nov. 11, 2014, 2:37 p.m. UTC | #2 
On Sat, 2014-11-08 at 16:01 +0100, Richard Henderson wrote:
> On 11/08/2014 03:20 PM, Alexandre Oliva wrote:
> > ctermid: return string literal, document MT-Safety pitfall
> > 
> > From: Alexandre Oliva <aoliva@redhat.com>
> > 
> > for  ChangeLog
> > 
> > 	* sysdeps/posix/ctermid.c (ctermid): Return a pointer to a
> > 	string literal if not passed a buffer.
> > 	* manual/job.texi (ctermid): Update reasoning, note deviation
> > 	from posix, suggest mtasurace when not passed a buffer, for
> > 	future non-preliminary safety notes.
> 
> LGTM.

Same for me.
diff mbox

Patch

diff --git a/manual/job.texi b/manual/job.texi
index 4f9bd81..095c26d 100644
--- a/manual/job.texi
+++ b/manual/job.texi
@@ -1039,10 +1039,12 @@  The function @code{ctermid} is declared in the header file
 @comment stdio.h
 @comment POSIX.1
 @deftypefun {char *} ctermid (char *@var{string})
-@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
+@safety{@prelim{}@mtsafe{@mtsposix{/!string}}@assafe{}@acsafe{}}
 @c This function is a stub by default; the actual implementation, for
-@c posix systems, returns an internal buffer if passed a NULL string,
-@c but the internal buffer is always set to /dev/tty.
+@c posix systems, returns a pointer to a string literal if passed a NULL
+@c string.  It's not clear we want to commit to being MT-Safe in the
+@c !string case, so maybe add mtasurace{:ctermid/!string} when we take
+@c prelim out, to make room for using a static buffer in the future.
 The @code{ctermid} function returns a string containing the file name of
 the controlling terminal for the current process.  If @var{string} is
 not a null pointer, it should be an array that can hold at least
diff --git a/sysdeps/posix/ctermid.c b/sysdeps/posix/ctermid.c
index 0ef9a3f..9714285 100644
--- a/sysdeps/posix/ctermid.c
+++ b/sysdeps/posix/ctermid.c
@@ -19,17 +19,17 @@ 
 #include <string.h>
 
 
-/* Return the name of the controlling terminal.
-   If S is not NULL, the name is copied into it (it should be at
-   least L_ctermid bytes long), otherwise a static buffer is used.  */
+/* Return the name of the controlling terminal.  If S is not NULL, the
+   name is copied into it (it should be at least L_ctermid bytes
+   long), otherwise we return a pointer to a non-const but read-only
+   string literal, that POSIX states the caller must not modify.  */
 char *
-ctermid (s)
-     char *s;
+ctermid (char *s)
 {
-  static char name[L_ctermid];
+  char *name = (char /*drop const*/ *) "/dev/tty";
 
   if (s == NULL)
-    s = name;
+    return name;
 
-  return strcpy (s, "/dev/tty");
+  return strcpy (s, name);
 }