Issue 29680706: Noissue - zero length OwnedString is valid.

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Issue 29680706: Noissue - zero length OwnedString is valid. (Closed)

Created:
Jan. 26, 2018, 8:33 p.m. by hub

Modified:
Jan. 26, 2018, 11:07 p.m.

Reviewers:
sergei

Base URL:
https://hg.adblockplus.org/adblockpluscore/

Visibility:
Public.

Description

Noissue - zero length OwnedString is valid.

Patch Set 1 #

Created: Jan. 26, 2018, 8:33 p.m.

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+3 lines, -1 line)			Patch
	M	compiled/String.h	View		1 chunk	+3 lines, -1 line	0 comments	Download

Messages

Total messages: 5

Expand All Messages | Collapse All Messages

hub

Issue #6138 looks familiar. This is required for OwnedStringMap<> to work with "" as a ...

Jan. 26, 2018, 8:34 p.m. (2018-01-26 20:34:55 UTC) #2

hub

I have an upcoming patch with (native) tests where I have integrated this into the ...

Jan. 26, 2018, 8:43 p.m. (2018-01-26 20:43:44 UTC) #3

sergei

LGTM I'm not sure that `| READ_WRITE` is needed because it's zero and inconsistent with ...

Jan. 26, 2018, 10:04 p.m. (2018-01-26 22:04:57 UTC) #4

hub

Jan. 26, 2018, 11:04 p.m. (2018-01-26 23:04:03 UTC) #5

On 2018/01/26 22:04:57, sergei wrote:
> LGTM
> 
> I'm not sure that `| READ_WRITE` is needed because it's zero and inconsistent
> with other code, but in general it's correct.

Without the READ_WRITE I get lots of asserts. Also it is consistent with any
other OwnedString constructor where the exception is the invalid string.

We can work on fixing that further down the road.

Expand All Messages | Collapse All Messages