The default bug view has changed. See this FAQ.

accessible objects should have private copy constructor

RESOLVED FIXED in mozilla11

Status

()

Core
Disability Access APIs
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: surkov, Assigned: jhk)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla11
access
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

7 years ago
Accessible objects should define copy constructor and assignment operators in private section to avoid accident calls (internal or on AT side). It was inspired by talk with Pete.
(Reporter)

Comment 1

5 years ago
check all classes in folder http://mxr.mozilla.org/mozilla-central/source/accessible/src/
Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++]
it appears you only need to check nsAccessNode and  not any of the things that inherit from it because having a private copy constructor / operator = means  things inheriting from you can't copy you.  Here's a test with gcc 4.6

tbsaunde@football:/tmp$ cat test.cpp
class foo
{
  public:
    foo() { }
  private:
  foo& operator =(const foo& aValue);
  foo(const foo&);
};

class dir : public foo
{
  public:
  dir() : foo() { }
  int mType;
};

  int main()
{
  dir bar;
  dir baz = bar;
  return baz.mType;
}

tbsaunde@football:/tmp$ g++ test.cpp
test.cpp: In copy constructor ‘dir::dir(const dir&)’:
test.cpp:7:3: error: ‘foo::foo(const foo&)’ is private
test.cpp:10:7: error: within this context
test.cpp: In function ‘int main()’:
test.cpp:20:13: note: synthesized method ‘dir::dir(const dir&)’ first required here
tbsaunde@football:/tmp$

sorry about the weird characters in the error message, it would appear something between bugzilla and my terminal dislikes utf8 :/
(Assignee)

Comment 3

5 years ago
Created attachment 581202 [details] [diff] [review]
Introduced copy constructor

patch_1
Attachment #581202 - Flags: review?(trev.saunders)
(Assignee)

Comment 4

5 years ago
Created attachment 581205 [details] [diff] [review]
Patch_2

.
Attachment #581202 - Attachment is obsolete: true
Attachment #581205 - Flags: review?(trev.saunders)
Attachment #581202 - Flags: review?(trev.saunders)
Comment on attachment 581205 [details] [diff] [review]
Patch_2

r=tbsaunde not surkov
Attachment #581205 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 6

5 years ago
Created attachment 581211 [details] [diff] [review]
Introduced copy constructor

r = tbsaunde
Attachment #581211 - Flags: review?(trev.saunders)
Comment on attachment 581211 [details] [diff] [review]
Introduced copy constructor

review not needed since same patch.
Attachment #581211 - Flags: review?(trev.saunders)
https://hg.mozilla.org/integration/mozilla-inbound/rev/332cdb521dd6

THANKS!
actually, https://hg.mozilla.org/integration/mozilla-inbound/rev/54c446091d7c
 push race :/
Just to note, there's a new macro you should use to do this, MOZ_DELETE:

http://hg.mozilla.org/mozilla-central/annotate/3c321d2c9884/mfbt/Attributes.h#l106

It's just a matter of #include "mozilla/Attributes.h", then putting MOZ_DELETE at the ends of declarations like the ones in comment 9's URL.  This has the advantage of producing errors at compile time, not (potentially) at link time (if the code making the accidental copy is a friend of the class being copied).
https://hg.mozilla.org/mozilla-central/rev/54c446091d7c

Leaving open for comment 10.
Assignee: nobody → jigneshhk1992
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla11
(Reporter)

Comment 12

5 years ago
Jignesh, could you address comment #10 please?
(Assignee)

Comment 13

5 years ago
Created attachment 595446 [details] [diff] [review]
Thanks for Pointing out.
Attachment #581205 - Attachment is obsolete: true
Attachment #595446 - Flags: review?(surkov.alexander)
(Reporter)

Comment 14

5 years ago
Comment on attachment 595446 [details] [diff] [review]
Thanks for Pointing out.

Review of attachment 595446 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, thank you for doing this
Attachment #595446 - Flags: review?(surkov.alexander) → review+
(Reporter)

Comment 15

5 years ago
inbound land https://hg.mozilla.org/integration/mozilla-inbound/rev/c6dda929759e
https://hg.mozilla.org/mozilla-central/rev/c6dda929759e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.