POSIX Proess Creation and Destruction nit-picking

POSIX Proess Creation and Destruction nit-picking

[Elad Lahav]

The following issues may not be deemed important enough to fix, but
I'll point them out nevertheless:

- kill() does not destroy a process, but merely sends a signal. The
action taken by the process upon the delivery of the signal depends on
the signal itself, whether it is currently masked, and whether the
process has installed a signal handler. The only signal guaranteed to
destroy a process is SIGKILL, which may not be masked and cannot be
associated with a handler.
- "It is critically important to note that the parent and child do not
share memory." - That is not true. Parent and child share all memory
marked as shared (i.e., mappings created with mmap(MAP_SHARED)). The
example works because by default the BSS is mapped as private.
- Listing 4.2 could have just used waitpid() instead of the loop.

If I were really a nit-picking person I would have pointed out the
various places in chapters 2 and 3 where infinitives are being split,
but I'm not so I won't ;-).

--Elad

Re: POSIX Proess Creation and Destruction nit-picking

[Akira Yokosawa]

Hello Elad,

On Fri, 26 Apr 2019 07:23:20 -0400, Elad Lahav wrote:
> The following issues may not be deemed important enough to fix, but
> I'll point them out nevertheless:
> 
[...]
> - Listing 4.2 could have just used waitpid() instead of the loop.

Listing 4.2 shows our implementation of waitall() function used under
CodeSamples/ directory of perfbook's repository.
Your feedback prompted me to review the listing and I noticed that
the name of the function is missing there.

I fixed the position of extraction markers in api-pthreads.h 

Please find the patch below.

> If I were really a nit-picking person I would have pointed out the
> various places in chapters 2 and 3 where infinitives are being split,
> but I'm not so I won't ;-).

As far as I am concerned, I like the colloquial tone of perfbook
and have no trouble seeing split infinitives as long as they are easy
to understand. Just a non-native speaker's opinion.

        Thanks, Akira

> --Elad
> 

----------8<------------------
From fec0187d2cb12fdf13a8c040c447c37c1bd5e183 Mon Sep 17 00:00:00 2001
From: Akira Yokosawa <akiyks@gmail.com>
Date: Sun, 28 Apr 2019 00:41:02 +0900
Subject: [PATCH] toolsoftrade: Fix extraction range of waitall()

Commit 29d0c7c185f2 ("toolsoftrade: Add labels in code samples as
comments") failed to include the function name and the closing brace
in the snippet of waitall() extracted from api-pthreads.h.

The original snippet removed in commit a62cf8469280 ("toolsoftrade:
Reference line in code snippets by label") contained those
lines.

Restore them by repositioning meta commands in api-pthreads.h

Reported-by: Elad Lahav <e2lahav@gmail.com>
Fixes: 29d0c7c185f2 ("toolsoftrade: Add labels in code samples as comments")
Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
---
 CodeSamples/api-pthreads/api-pthreads.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/CodeSamples/api-pthreads/api-pthreads.h b/CodeSamples/api-pthreads/api-pthreads.h
index f5510f01..5d17a9bd 100644
--- a/CodeSamples/api-pthreads/api-pthreads.h
+++ b/CodeSamples/api-pthreads/api-pthreads.h
@@ -287,9 +287,9 @@ static __inline__ void wait_all_threads(void)
 /*
  * Wait on all child processes.
  */
+// \begin{snippet}[labelbase=ln:api-pthreads:api-pthreads:waitall,commandchars=\%\[\]]
 static __inline__ void waitall(void)
 {
-// \begin{snippet}[labelbase=ln:api-pthreads:api-pthreads:waitall,commandchars=\%\[\]]
 	int pid;
 	int status;
 
@@ -303,8 +303,8 @@ static __inline__ void waitall(void)
 		}
 		poll(NULL, 0, 1);		//\fcvexclude
 	}					//\lnlbl{loopb}
-// \end{snippet}
 }
+// \end{snippet}
 
 static __inline__ void run_on(int cpu)
 {
-- 
2.17.1

Re: POSIX Proess Creation and Destruction nit-picking

[Akira Yokosawa]

On Sun, 28 Apr 2019 01:19:25 +0900, Akira Yokosawa wrote:
> Hello Elad,
> 
> On Fri, 26 Apr 2019 07:23:20 -0400, Elad Lahav wrote:
>> The following issues may not be deemed important enough to fix, but
>> I'll point them out nevertheless:
>>
> [...]
>> - Listing 4.2 could have just used waitpid() instead of the loop.
> 
> Listing 4.2 shows our implementation of waitall() function used under
> CodeSamples/ directory of perfbook's repository.
> Your feedback prompted me to review the listing and I noticed that
> the name of the function is missing there.
> 
> I fixed the position of extraction markers in api-pthreads.h 
> 
> Please find the patch below.

Hi Elad,

I might not be clear enough, but could you let me know if you think
the patch looks reasonable.  I mean, if you think this change is not
related to your feed back, I'll drop the Reported-by tag.

        Thanks, Akira

> 
>> If I were really a nit-picking person I would have pointed out the
>> various places in chapters 2 and 3 where infinitives are being split,
>> but I'm not so I won't ;-).
> 
> As far as I am concerned, I like the colloquial tone of perfbook
> and have no trouble seeing split infinitives as long as they are easy
> to understand. Just a non-native speaker's opinion.
> 
>         Thanks, Akira
> 
>> --Elad
>>
> 
> ----------8<------------------
> From fec0187d2cb12fdf13a8c040c447c37c1bd5e183 Mon Sep 17 00:00:00 2001
> From: Akira Yokosawa <akiyks@gmail.com>
> Date: Sun, 28 Apr 2019 00:41:02 +0900
> Subject: [PATCH] toolsoftrade: Fix extraction range of waitall()
> 
> Commit 29d0c7c185f2 ("toolsoftrade: Add labels in code samples as
> comments") failed to include the function name and the closing brace
> in the snippet of waitall() extracted from api-pthreads.h.
> 
> The original snippet removed in commit a62cf8469280 ("toolsoftrade:
> Reference line in code snippets by label") contained those
> lines.
> 
> Restore them by repositioning meta commands in api-pthreads.h
> 
> Reported-by: Elad Lahav <e2lahav@gmail.com>
> Fixes: 29d0c7c185f2 ("toolsoftrade: Add labels in code samples as comments")
> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
> ---
>  CodeSamples/api-pthreads/api-pthreads.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/CodeSamples/api-pthreads/api-pthreads.h b/CodeSamples/api-pthreads/api-pthreads.h
> index f5510f01..5d17a9bd 100644
> --- a/CodeSamples/api-pthreads/api-pthreads.h
> +++ b/CodeSamples/api-pthreads/api-pthreads.h
> @@ -287,9 +287,9 @@ static __inline__ void wait_all_threads(void)
>  /*
>   * Wait on all child processes.
>   */
> +// \begin{snippet}[labelbase=ln:api-pthreads:api-pthreads:waitall,commandchars=\%\[\]]
>  static __inline__ void waitall(void)
>  {
> -// \begin{snippet}[labelbase=ln:api-pthreads:api-pthreads:waitall,commandchars=\%\[\]]
>  	int pid;
>  	int status;
>  
> @@ -303,8 +303,8 @@ static __inline__ void waitall(void)
>  		}
>  		poll(NULL, 0, 1);		//\fcvexclude
>  	}					//\lnlbl{loopb}
> -// \end{snippet}
>  }
> +// \end{snippet}
>  
>  static __inline__ void run_on(int cpu)
>  {
> 

Re: POSIX Proess Creation and Destruction nit-picking

[Elad Lahav]

Hi Akira,

The patch looks fine, but is indeed unrelated to my comments.

--Elad

On Sat, 27 Apr 2019 at 19:00, Akira Yokosawa <akiyks@gmail.com> wrote:
>
> On Sun, 28 Apr 2019 01:19:25 +0900, Akira Yokosawa wrote:
> > Hello Elad,
> >
> > On Fri, 26 Apr 2019 07:23:20 -0400, Elad Lahav wrote:
> >> The following issues may not be deemed important enough to fix, but
> >> I'll point them out nevertheless:
> >>
> > [...]
> >> - Listing 4.2 could have just used waitpid() instead of the loop.
> >
> > Listing 4.2 shows our implementation of waitall() function used under
> > CodeSamples/ directory of perfbook's repository.
> > Your feedback prompted me to review the listing and I noticed that
> > the name of the function is missing there.
> >
> > I fixed the position of extraction markers in api-pthreads.h
> >
> > Please find the patch below.
>
> Hi Elad,
>
> I might not be clear enough, but could you let me know if you think
> the patch looks reasonable.  I mean, if you think this change is not
> related to your feed back, I'll drop the Reported-by tag.
>
>         Thanks, Akira
>
> >
> >> If I were really a nit-picking person I would have pointed out the
> >> various places in chapters 2 and 3 where infinitives are being split,
> >> but I'm not so I won't ;-).
> >
> > As far as I am concerned, I like the colloquial tone of perfbook
> > and have no trouble seeing split infinitives as long as they are easy
> > to understand. Just a non-native speaker's opinion.
> >
> >         Thanks, Akira
> >
> >> --Elad
> >>
> >
> > ----------8<------------------
> > From fec0187d2cb12fdf13a8c040c447c37c1bd5e183 Mon Sep 17 00:00:00 2001
> > From: Akira Yokosawa <akiyks@gmail.com>
> > Date: Sun, 28 Apr 2019 00:41:02 +0900
> > Subject: [PATCH] toolsoftrade: Fix extraction range of waitall()
> >
> > Commit 29d0c7c185f2 ("toolsoftrade: Add labels in code samples as
> > comments") failed to include the function name and the closing brace
> > in the snippet of waitall() extracted from api-pthreads.h.
> >
> > The original snippet removed in commit a62cf8469280 ("toolsoftrade:
> > Reference line in code snippets by label") contained those
> > lines.
> >
> > Restore them by repositioning meta commands in api-pthreads.h
> >
> > Reported-by: Elad Lahav <e2lahav@gmail.com>
> > Fixes: 29d0c7c185f2 ("toolsoftrade: Add labels in code samples as comments")
> > Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
> > ---
> >  CodeSamples/api-pthreads/api-pthreads.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/CodeSamples/api-pthreads/api-pthreads.h b/CodeSamples/api-pthreads/api-pthreads.h
> > index f5510f01..5d17a9bd 100644
> > --- a/CodeSamples/api-pthreads/api-pthreads.h
> > +++ b/CodeSamples/api-pthreads/api-pthreads.h
> > @@ -287,9 +287,9 @@ static __inline__ void wait_all_threads(void)
> >  /*
> >   * Wait on all child processes.
> >   */
> > +// \begin{snippet}[labelbase=ln:api-pthreads:api-pthreads:waitall,commandchars=\%\[\]]
> >  static __inline__ void waitall(void)
> >  {
> > -// \begin{snippet}[labelbase=ln:api-pthreads:api-pthreads:waitall,commandchars=\%\[\]]
> >       int pid;
> >       int status;
> >
> > @@ -303,8 +303,8 @@ static __inline__ void waitall(void)
> >               }
> >               poll(NULL, 0, 1);               //\fcvexclude
> >       }                                       //\lnlbl{loopb}
> > -// \end{snippet}
> >  }
> > +// \end{snippet}
> >
> >  static __inline__ void run_on(int cpu)
> >  {
> >
>

[PATCH v2] toolsoftrade: Fix extraction range of waitall()

[Akira Yokosawa]

From 23e640facc8198d97ec712e5b79aa3a69a270eb7 Mon Sep 17 00:00:00 2001
From: Akira Yokosawa <akiyks@gmail.com>
Date: Sun, 28 Apr 2019 00:41:02 +0900
Subject: [PATCH v2] toolsoftrade: Fix extraction range of waitall()

Commit 29d0c7c185f2 ("toolsoftrade: Add labels in code samples as
comments") failed to include the function name and the closing brace
in the snippet of waitall() extracted from api-pthreads.h.

The original snippet removed in commit a62cf8469280 ("toolsoftrade:
Reference line in code snippets by label") contained those
lines.

Fix it by moving meta commands in api-pthreads.h

Fixes: 29d0c7c185f2 ("toolsoftrade: Add labels in code samples as comments")
Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
---
v1 -> v2: As per Elad's POV, remove Reported-by tag.

 CodeSamples/api-pthreads/api-pthreads.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/CodeSamples/api-pthreads/api-pthreads.h b/CodeSamples/api-pthreads/api-pthreads.h
index f5510f01..5d17a9bd 100644
--- a/CodeSamples/api-pthreads/api-pthreads.h
+++ b/CodeSamples/api-pthreads/api-pthreads.h
@@ -287,9 +287,9 @@ static __inline__ void wait_all_threads(void)
 /*
  * Wait on all child processes.
  */
+// \begin{snippet}[labelbase=ln:api-pthreads:api-pthreads:waitall,commandchars=\%\[\]]
 static __inline__ void waitall(void)
 {
-// \begin{snippet}[labelbase=ln:api-pthreads:api-pthreads:waitall,commandchars=\%\[\]]
 	int pid;
 	int status;
 
@@ -303,8 +303,8 @@ static __inline__ void waitall(void)
 		}
 		poll(NULL, 0, 1);		//\fcvexclude
 	}					//\lnlbl{loopb}
-// \end{snippet}
 }
+// \end{snippet}
 
 static __inline__ void run_on(int cpu)
 {
-- 
2.17.1

Re: [PATCH v2] toolsoftrade: Fix extraction range of waitall()

[Paul E. McKenney]

On Tue, Apr 30, 2019 at 09:12:16AM +0900, Akira Yokosawa wrote:
> >From 23e640facc8198d97ec712e5b79aa3a69a270eb7 Mon Sep 17 00:00:00 2001
> From: Akira Yokosawa <akiyks@gmail.com>
> Date: Sun, 28 Apr 2019 00:41:02 +0900
> Subject: [PATCH v2] toolsoftrade: Fix extraction range of waitall()
> 
> Commit 29d0c7c185f2 ("toolsoftrade: Add labels in code samples as
> comments") failed to include the function name and the closing brace
> in the snippet of waitall() extracted from api-pthreads.h.
> 
> The original snippet removed in commit a62cf8469280 ("toolsoftrade:
> Reference line in code snippets by label") contained those
> lines.
> 
> Fix it by moving meta commands in api-pthreads.h
> 
> Fixes: 29d0c7c185f2 ("toolsoftrade: Add labels in code samples as comments")
> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>

Queued and pushed, thank you!

							Thanx, Paul

> ---
> v1 -> v2: As per Elad's POV, remove Reported-by tag.
> 
>  CodeSamples/api-pthreads/api-pthreads.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/CodeSamples/api-pthreads/api-pthreads.h b/CodeSamples/api-pthreads/api-pthreads.h
> index f5510f01..5d17a9bd 100644
> --- a/CodeSamples/api-pthreads/api-pthreads.h
> +++ b/CodeSamples/api-pthreads/api-pthreads.h
> @@ -287,9 +287,9 @@ static __inline__ void wait_all_threads(void)
>  /*
>   * Wait on all child processes.
>   */
> +// \begin{snippet}[labelbase=ln:api-pthreads:api-pthreads:waitall,commandchars=\%\[\]]
>  static __inline__ void waitall(void)
>  {
> -// \begin{snippet}[labelbase=ln:api-pthreads:api-pthreads:waitall,commandchars=\%\[\]]
>  	int pid;
>  	int status;
>  
> @@ -303,8 +303,8 @@ static __inline__ void waitall(void)
>  		}
>  		poll(NULL, 0, 1);		//\fcvexclude
>  	}					//\lnlbl{loopb}
> -// \end{snippet}
>  }
> +// \end{snippet}
>  
>  static __inline__ void run_on(int cpu)
>  {
> -- 
> 2.17.1
> 
>