Re: [PATCH V2] fgetgrent.3: ATTRIBUTES: Note function that is thread-safe

On 02/12/2015 04:22 PM, Michael Kerrisk (man-pages) wrote:

Hello Ma Shimiao,

On 02/12/2015 02:24 AM, Ma Shimiao wrote:

quoted

Hi Michael,
On 02/11/2015 06:20 PM, Michael Kerrisk (man-pages) wrote:

quoted

Hi Ma Shimiao,

On 11 February 2015 at 10:38, Ma Shimiao [off-list ref] wrote:

quoted

On 02/11/2015 04:28 PM, Michael Kerrisk (man-pages) wrote:

quoted

Hi Ma Shmiao,

On 11 February 2015 at 09:20, Ma Shimiao [off-list ref] wrote:

quoted

Hi Michael,
On 02/11/2015 03:55 PM, Michael Kerrisk (man-pages) wrote:

quoted

On 02/11/2015 08:35 AM, Ma Shimiao wrote:

quoted

The marking matches glibc marking.

Signed-off-by: Ma Shimiao <redacted>
---
 man3/fgetgrent.3 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/man3/fgetgrent.3 b/man3/fgetgrent.3
index 57665dd..e599483 100644
--- a/man3/fgetgrent.3
+++ b/man3/fgetgrent.3

@@ -90,6 +90,18 @@ is set to indicate the cause.
 Insufficient memory to allocate
 .I group
 structure.
+.SH ATTRIBUTES
+For an explanation of the terms used in this section, see
+.BR attributes (7).
+.TS
+allbox;
+lb lb lb
+l l l.
+Interface   Attribute       Value
+T{
+.BR fgetgrent ()
+T}  Thread safety   MT-Unsafe race:fgrent

Why the change here in the V2? What does "fgrent" refer to?

race:fgrent is right mark, I made a mistake.
race:fgrent is similar to race:grent.
race:grent is used to indicate data race exists in getgrent(), setgrent() and endgrent().
race:fgrent is used to indicate data race exists when using fgetgrent() in multi-thread.

My question then is: how does the reader know that "grent" refers to
"getgrent(), setgrent() and endgrent()"?

From definition of race in glibc manual, we get:
If data race exists and objects cause data race are not from user,
then the function should be annotated as MT-Unsafe and marked with race.
And we need a colon and an identifier follows race to tell user what causes data race.

getgrent(), setgrent() and endgrent() are usually used together.
Because they need to share an iterator, then data race occurs between them.
We want users to know when using them together in multi-thread, data race makes them
unsafe in multi-thread. But, we can't definitely write which internal object causes data race.
So, we extract the common string 'grent' from functions' name as a identifier which groups
getgrent(), setgrent() and endgrent() to tell users that the group of *grent() functions
can't be used together in multi-thread.

All of the above is fine. But:

quoted

If a reader understand the definition of race, I think he can know that "grent" refers to
getgrent(), setgrent() and endgrent().

I think many readers will not be able to deduce this. (And I think
readers of the glibc manual will have exactly the same problem.) I
think we somehow need to make this a bit clearer, perhaps in a
sentence following the table. Would you have a proposal for such a
sentence?

How about the following sentence?
"grent" in "race:grent" is an identifier which groups functions xxxgrent() used to remind that
if functions xxxgrent() were used together in multi-thread, data race would occur.

I think that's a good start, but I'd prefer something a little more explicit:

[[
In the above table,
.I grent in
.I race:grent
signifies that if any of the functions
.BR setgrent (),
.BR getgrent (),
or
.BR endgrent ()
are used in parallel in different threads of a program, then data races could occur.
]]

How would that be?

That looks fine. I will add them in new patch.

Also, what is the analogous text for fgetgrent() / "fgrent"? Is the problem
races between different threads using fgetgrent() or is it races with another 
thread using setgrent/getgrent/sendgrent? If the former, I don't understand 
why we need the identifier "fgrent" instead of just using "fgetgrent".

The problem is the former.
At first, I chose fgetgrent as identifier.
Then, I found fgrent is used in glib manual.
As fgetgrent looks like getgrent(), I thought make identifier of race to be
similar is not a bad idea.
At the end, I modified fgetgrent to fgrent.

Now, after thinking for while, fgetgrent seems more suitable.
I‘m sorry for sending patches in haste.
I haven't asked glibc community why they choose to use fgrent.
After talking with them, I will send a new patch.
Is it OK?


Thanks

Thanks,

Michael

-- 
Ma Shimiao
Development Dept.I
Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html