Opened 10 years ago

Closed 10 years ago

#94 closed task (fixed)

review: truncation support in the DNS message library

Reported by: jinmei Owned by: jinmei
Priority: very high Milestone: 03. 1st Incremental Release
Component: libdns++ Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: Add Hours to Ticket:
Total Hours: Internal?:

Description

There are some additional changes to code already reviewed to support message truncation in rendering. This part should be reviewed.

The additional changes are pretty minor and I believe they can be easily reviewed.

Note: the truncation logic itself may need to be revisited post year1. This review is specifically for the year1 deliverable.

The change set can be retrieved by
% svn diff -r 1120:1456 name.h name.cc tests/name_unittest.cc
at the trunk/src/lib/dns directory.

Subtickets

Change History (5)

comment:1 Changed 10 years ago by jinmei

  • Owner changed from jinmei to shane
  • Status changed from new to assigned

assigning it to Shane as the "review dispatcher".

comment:2 follow-up: Changed 10 years ago by shane

  • Owner changed from shane to jelte

Review should be straightforward. It might not still be applicable, but Jinmei can let us know in that case.

comment:3 in reply to: ↑ 2 Changed 10 years ago by jinmei

Replying to shane:

Review should be straightforward. It might not still be applicable, but Jinmei can let us know in that case.

I don't know what you mean by "(not) applicable"...but we won't have to revisit the design for the next week's release, so it would be nice to get the current code reviewed.

comment:4 follow-up: Changed 10 years ago by jelte

  • Owner changed from jelte to jinmei

So apart from replacing that non-local static by fixed arrays for digitvalue and maptolower the changes are only in the throw() statements?

Both of these look good, although the documentation for Name(Buffer) has not been updated.

I have not checked the exception catching in calling functions, if this is needed to, I will.

comment:5 in reply to: ↑ 4 Changed 10 years ago by jinmei

  • Resolution set to fixed
  • Status changed from assigned to closed

Replying to jelte:

So apart from replacing that non-local static by fixed arrays for digitvalue and maptolower the changes are only in the throw() statements?

Yes, and one more minor point: removed unnecessary const from the return type (when an object is returned):

  • const uint8_t at(size_t pos) const

+ uint8_t at(size_t pos) const

Both of these look good, although the documentation for Name(Buffer) has not been updated.

Okay, thanks. And the doc error was fixed in r1736.

I have not checked the exception catching in calling functions, if this is needed to, I will.

Do you mean other than unittests? If so, no I don't think you need to check them.

Thanks for the review. Closing ticket.

Note: See TracTickets for help on using tickets.