Neel Basu
Neel Basu

Reputation: 12904

stream >> reading last line twice

I am trying to parse /proc/partitions file.

major minor  #blocks  name

   8        0  976762584 sda
   8        1   99998720 sda1
   8        2          1 sda2
   8        3  103561216 sda3
   8        4  291514368 sda4
   8        5    1998848 sda5

This is the /proc/partitions file in my machine.

#include <boost/cstdint.hpp>
#include <fstream>
#include <boost/algorithm/string/trim.hpp>
#include <boost/format.hpp>

int main(){
  std::ifstream proc_partitions_stream("/proc/partitions");
  boost::int32_t disc_partition_line_count = -2; //-1 for headers, -1 for the empty line
  //so counter is 0 when it tries to read the real entries
  while(!proc_partitions_stream.fail()){
    if(disc_partition_line_count >= 0){
      boost::uint16_t   major, minor;
      boost::uint64_t   blocks;
      std::string   label;
      proc_partitions_stream >> major >> minor >> blocks >> label;
      std::cout << boost::format("%1% %2% %3% %4%") % major % minor % blocks % label << std::endl;
      boost::algorithm::trim(label);      
    }else{
      std::string line;
      std::getline(proc_partitions_stream, line);
    }
    ++disc_partition_line_count;
  }
    return 0;
}

But it reads the last line twice Here is the program

8 0 976762584 [sda]
8 1 99998720 [sda1]
8 2 1 [sda2]
8 3 103561216 [sda3]
8 4 291514368 [sda4]
8 5 1998848 [sda5]
8 5 1998848 [] << read the last line TWICE but didn't read the label

Upvotes: 1

Views: 1667

Answers (2)

Jerry Coffin
Jerry Coffin

Reputation: 490808

I would rewrite it to something more like:

#include <boost/cstdint.hpp>
#include <fstream>
#include <boost/algorithm/string/trim.hpp>
#include <boost/format.hpp>

int main(){
  std::ifstream proc_partitions_stream("/proc/partitions");

  for (int i=0; i<2; i++)
      proc_partitions_stream.ignore(max_len, '\n');

  boost::uint16_t   major, minor;
  boost::uint64_t   blocks;
  std::string   label;
  while (proc_partitions_stream >> major >> minor >> blocks >> label) {
      std::cout << boost::format("%1% %2% %3% %4%") % major % minor % blocks % label << "\n";
      //boost::algorithm::trim(label); // was present, but non-functional?
  }
  return 0;
}

Alternatively, define a small class to represent a disc partition, and overload operator>> and << for that, as well as a small function to skip lines from an istream:

class partition { 
    boost::uint16_t major, minor;
    boost uint64_t  blocks;
    std::string     label;
public:
     friend std::istream &operator>>(std::istream &is, partition &p) { 
         return is >> major >> minor >> blocks >> label;
     }

     friend std::ostream &operator<<(std::ostream &os, partition const &p) { 
         return os << boost::format("%1% %2% %3% %4%") % major % minor % blocks % label;
     }
};

std::istream &skiplines(std::istream &is, unsigned count) { 
   unsigned max_len = something; // see below
   return is.ignore(max_len, '\n');
}

Then in main you'd just have something like:

if (!skiplines(2)) {
    std::cerr << "Error!\n";
    return 1;
}

std::copy(std::istream_iterator<partition>(proc_partitions_stream),
          std::istream_iterator<partition>(),
          std::ostream_iterator<partition>(std::cout, "\n"));

As far as what value to use for max_len goes: quite a few people use std::numeric_limits<std::streamsize>::max(). I prefer something quite a bit smaller as a rule. It probably doesn't make any difference in this case (chances or mis-formatted input are small) but if you're just trying to skip a line, it's better to limit it to some amount that's at least halfway reasonable for a line. If you're just going to tell the user there's a problem anyway, there's no reason to waste their time waiting for you to read gigabytes of garbage before you do so.

Upvotes: 2

john
john

Reputation: 88092

Because your while loop condition is wrong, you test for failure before you read, you should test for failure after you read. Even better would be to test the return from getline.

Upvotes: 3

Related Questions