[CRIU] [PATCH 1/2] proc: add vma_area in a list after parsing all parameters

[Cyrill Gorcunov]

On Tue, May 21, 2013 at 04:15:48PM +0400, Andrew Vagin wrote:
> On Tue, May 21, 2013 at 03:09:15PM +0400, Cyrill Gorcunov wrote:
> > On Tue, May 21, 2013 at 02:37:52PM +0400, Andrey Vagin wrote:
> > >  
> > > -	while (fgets(buf, BUF_SIZE, smaps)) {
> > > +	while (1) {
> > >  		int num;
> > >  		char file_path[6];
> > > +		bool eof;
> > > +
> > > +		eof = (fgets(buf, BUF_SIZE, smaps) == NULL);
> > >  
> > > -		if (!is_vma_range_fmt(buf)) {
> > > +		if (!eof && !is_vma_range_fmt(buf)) {
> > >  			if (!strncmp(buf, "Nonlinear", 9)) {
> > >  				BUG_ON(!vma_area);
> > >  				pr_err("Nonlinear mapping found %016"PRIx64"-%016"PRIx64"\n",
> > > @@ -190,6 +193,21 @@ int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list, bool use_map_file
> > >  				continue;
> > >  		}
> > >  
> > 
> > Andrew, correct me please, if I'm mistaken -- the vma_area is the last vma
> > allocated, iow it's last from the &vma_area_list->h, right? Thus maybe
> > we could do this handling out of cycle at all instead of this eof game?
> 
> I don't understand what do you mean. This loop handles all vma for a
> process.

Well, I must admit I don't quite follow what you've changed here. You've added
eof flag which breaks the loop and adds vma to the list. How this is different
from the original code where we have been testing for vma::status then
add vma to the list. Can't we do instead

done:
		list_add_tail(&vma_area->list, &vma_area_list->h);
		vma_area_list->nr++;
		if (privately_dump_vma(vma_area)) {
			unsigned long pages;

			pages = vma_area_len(vma_area) / PAGE_SIZE;
			vma_area_list->priv_size += pages;
			vma_area_list->longest = max(vma_area_list->longest, pages);
		}
		vma_area = NULL;

This should be equivalent to what you did but without eof flag at all. No?