Re: [RFA] Linux Checkpoints, take 3

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Fri, 23 Dec 2005 14:41:50 -0800
> From: Michael Snyder <msnyder@redhat.com>
> 
> This is a multi-part message in MIME format.
> --------------000904040807050504080802
> Content-Type: text/plain; charset=ISO-8859-1; format=flowed
> Content-Transfer-Encoding: 7bit
> 
> Fixups per Daniel and Eli.   Daniel,
> 1) Regarding register_raw_read, I just sidestepped the issue
> and added a PC field to my saved context.
> 2) Regarding threads, I could make checkpoint_command refuse
> if there are threads -- what's the best way to check that?
> I could look at to_shortname, or better, I could add an
> access method for the static variable "threaded" in linux-nat.
> 
> Mark, I'd still like to have your ok.

Sorry Michael, been to busy hacking other things.  But here we go:

> +       /* Now save the 'state' (file position) of all open file descriptors.
> + 	 Unfortunately fork does not take care of that for us...  */

Hmm, fork(2) clones it file descroptors but both file descriptors
refer to the same open file description.  Nasty.  Wonder whether it'd
be possible to copy the open file descriptorions too.  Well, I guess
the point was to do this without kernel support...

Anyway, a few problems that I spotted:

1. Please drop the "extern" from _initialize_linux_fork() (and the
   associated  comment).  You'll need an explicit prototype again, like:

   /* Prevent warning from -Wmissing-prototypes.  */
   void _initialize_linux_fork (void);

2. Please update the copyright notice to use (C) and the new FSF
   address.

3. Replace the FORKS_EXISTS() macro with a forks_exists_p() function?
   Or inline it if you don't think it's ever going to be changed.

4. Aren't the values used for SEEK_SET/SEEK_CUR the same on all Linux
   ports?  In that case perhaps #defining LINUX_SEEK_SET 0, and using
   LINUX_SEEK_SET would be a good solution.

5. Could you seperate out the "Detaching after fork from child
   process" printing?

And from the nitpicking department:

1. The comments on the #includes are a bit silly.  I'd rather not see
   any unless there's something really unobvious going on.  They get
   out of data pretty soon anyway.

2. Please don't put the function name in comments above the function.
   (call_lseek, linux_fork_killall, linux_fork_mourn_inferior, 

3. Really sorry to hear that you're suffering from a split personality
   problem:

> 2005-12-19  Michael Snyder  <msnyder@redhat.com>
> 
> 2005-12-19  Michael Van Meter Snyder  <michsnyd@clwang-lnx.cisco.com>
> 

Otherwise, I think it's ok.  It's a bit sad that this is a Linux-only
implementation, but then it's mostly Linux-specific code.  I'll see if
I can rig up something for OpenBSD, and then we can sort out what bits
are target-independent.

Mark