Limited NSEC support to RecordParsed

net/dns/record_rdata.h

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef NET_DNS_RECORD_RDATA_H_
 #define NET_DNS_RECORD_RDATA_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 163 matching lines @@
   const std::vector<std::string>& texts() const { return texts_; }
 
  private:
   TxtRecordRdata();
 
   std::vector<std::string> texts_;
 
   DISALLOW_COPY_AND_ASSIGN(TxtRecordRdata);
 };
 
+// Only the subset of the nsec record format required by mDNS is supported.
+// Nsec (limited) record format (http://www.ietf.org/rfc/rfc3845.txt and
+//                               http://www.rfc-editor.org/rfc/rfc6762.txt):
+// next-domain (ignored): A domain name.
+// one bit-map:

[szym, 2013/06/24 21:50:28]

The formatting suggests variable names, but they are not used like this in code
nor in the referenced RFCs. There is also no need for "one" in "one bit-map".

I suggest using:
"Next Domain Name"
and
"Type Bit Map"

or better yet copy the relevant part of RFC6762 here.

Another option is to leave this out (a reference to RFC seems sufficient) and
perhaps add the most useful remarks as comments in the implementation of Create.

[Noam Samuel, 2013/06/25 01:31:31]

Done.

+//     - one byte zeroed out to represent the zeroth window
+//     - one byte from 0-32 representing the length of the bitmap in bytes
+//     - the bitmap, each bit representing whether or not the record
+//     corresponding to the bit number exists for that domain.
+class NET_EXPORT_PRIVATE NsecRecordRdata : public RecordRdata {
+ public:
+  static const uint16 kType = dns_protocol::kTypeNSEC;
+
+  virtual ~NsecRecordRdata();
+  static scoped_ptr<NsecRecordRdata> Create(const base::StringPiece& data,
+                                            const DnsRecordParser& parser);
+  virtual bool IsEqual(const RecordRdata* other) const OVERRIDE;
+  virtual uint16 Type() const OVERRIDE;
+
+  const std::vector<uint8>& bitmap() { return bitmap_; }

[szym, 2013/06/24 21:50:28]

Do you need to expose both bitmap and bit?

[Noam Samuel, 2013/06/25 01:15:35]

The main use case for exposing bitmap is to allow users to copy the full bitmap
easily. I don't think this should be necessary for most uses of nsec (since nsec
should really only be used inside the mDNS layer itself, this is easier to say).
I think it makes more sense to expose only the size of the bitmap, in case the
user wants to do something for each bit.

+  bool bit(unsigned i);

[gene, 2013/06/24 19:47:16]

could you please add comment describing what bit() is doing?
also, consider naming it GetBit() since logic requires a few lines

[Noam Samuel, 2013/06/24 21:31:30]

Done.

+
+ private:
+  NsecRecordRdata();
+
+  std::vector<uint8> bitmap_;
+
+  DISALLOW_COPY_AND_ASSIGN(NsecRecordRdata);
+};
+
+
 }  // namespace net
 
 #endif  // NET_DNS_RECORD_RDATA_H_