update_engine: use librootdev

update_engine: use librootdev

Rewrote BootDevice() to call rootdev() with full resolution
enabled.  BootDevice() was crawling /proc/cmdline while rootdev()
uses /sys.  We should have both /proc and /sys normally so hopefully
this is a minor change to get us to a working updater over dm-verity.

TEST=none yet.
BUG=chromium-os:5938
Change-Id: I7ee654d5b429d67d65e9729210ef839c58fa1d34

[adlr, 2010-08-30 17:41:21]

LGTM w/ nit

http://codereview.chromium.org/3232004/diff/1/3
File utils.cc (right):

http://codereview.chromium.org/3232004/diff/1/3#newcode14
utils.cc:14: #include <rootdev/rootdev.h>
our new policy is the public c++ style guide:
- corresponding .h file
- c headers
- c++ headers
- lib headers
- project headers

so change it to:

#include "update_engine/utils.h"

#include <sys/mount.h>
#include <sys/resource.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

#include <algorithm>

#include <base/file_path.h>
#include <base/file_util.h>
#include <base/rand_util.h>
#include <base/string_util.h>
#include <base/logging.h>
#include <rootdev/rootdev.h>

#include "update_engine/file_writer.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/subprocess.h"

http://codereview.chromium.org/3232004/diff/1/3#newcode369
utils.cc:369: char boot_path[4096];
use PATH_MAX or something rather than a magic number

[petkov, 2010-08-30 17:45:00]

in addition to adlr's nits...

LGTM otherwise.

http://codereview.chromium.org/3232004/diff/1/3
File utils.cc (right):

http://codereview.chromium.org/3232004/diff/1/3#newcode368
utils.cc:368: string boot_device("");
isn't "" the default? actually, I'd just remove this declaration -- see below.

http://codereview.chromium.org/3232004/diff/1/3#newcode375
utils.cc:375: return boot_device;
return "";

http://codereview.chromium.org/3232004/diff/1/3#newcode380
utils.cc:380: return boot_device;
return boot_path;

[Will Drewry, 2010-08-30 17:59:08]

all comments integrated!

Doing a quick test locally now.

http://codereview.chromium.org/3232004/diff/1/3
File utils.cc (right):

http://codereview.chromium.org/3232004/diff/1/3#newcode14
utils.cc:14: #include <rootdev/rootdev.h>
On 2010/08/30 17:41:21, adlr wrote:
> our new policy is the public c++ style guide:
> - corresponding .h file
> - c headers
> - c++ headers
> - lib headers
> - project headers
> 
> so change it to:
> 
> #include "update_engine/utils.h"
> 
> #include <sys/mount.h>
> #include <sys/resource.h>
> #include <sys/stat.h>
> #include <sys/types.h>
> #include <dirent.h>
> #include <errno.h>
> #include <fcntl.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> 
> #include <algorithm>
> 
> #include <base/file_path.h>
> #include <base/file_util.h>
> #include <base/rand_util.h>
> #include <base/string_util.h>
> #include <base/logging.h>
> #include <rootdev/rootdev.h>
> 
> #include "update_engine/file_writer.h"
> #include "update_engine/omaha_request_params.h"
> #include "update_engine/subprocess.h"

Done.  Fixed up base as well.

http://codereview.chromium.org/3232004/diff/1/3#newcode368
utils.cc:368: string boot_device("");
On 2010/08/30 17:45:00, petkov wrote:
> isn't "" the default? actually, I'd just remove this declaration -- see below.

It's supposed to be, but I've been burned in the past with it not being.   Any
overhead should be optimized away regardless. 

Anyway, this code is dead.

http://codereview.chromium.org/3232004/diff/1/3#newcode369
utils.cc:369: char boot_path[4096];
On 2010/08/30 17:41:21, adlr wrote:
> use PATH_MAX or something rather than a magic number

Done.

http://codereview.chromium.org/3232004/diff/1/3#newcode375
utils.cc:375: return boot_device;
On 2010/08/30 17:45:00, petkov wrote:
> return "";
> 

Done.

http://codereview.chromium.org/3232004/diff/1/3#newcode380
utils.cc:380: return boot_device;
On 2010/08/30 17:45:00, petkov wrote:
> return boot_path;
> 

Done.  

I wasn't sure if g++ would properly call the string constructor first, but it
looks like it does.

[petkov, 2010-08-30 18:17:18]

still LGTM

http://codereview.chromium.org/3232004/diff/6001/4003
File utils.cc (right):

http://codereview.chromium.org/3232004/diff/6001/4003#newcode380
utils.cc:380: LOG(WARNING) << "rootdev found a device name with no device node";
You could use LOG_IF(WARNING, ret > 0) << ... Not sure if it improves
readability though :-)