[Webkit-unassigned] [Bug 113466] [Mac][WK2] Don’t let plug-ins use System V shared memory

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=113466


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #196056|commit-queue?               |commit-queue-
               Flag|                            |




--- Comment #4 from Alexey Proskuryakov <ap at webkit.org>  2013-04-01 22:05:14 PST ---
(From update of attachment 196056)
View in context: https://bugs.webkit.org/attachment.cgi?id=196056&action=review

This patch still looks great overall, and still needs many style fixes before it can be landed. Someone will need to make these fixes.

> Source/WebKit2/ChangeLog:3
> +        [Mac][WK2] Don’t let plug-ins use System V shared memory

This patch is also mangled. Are you using a Unicode aware text editor?

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:129
> +// This needs to C linkage as it is replacing C based system calls.
> +extern "C" {

This comment is not accurate. As evidenced by other functions we have shims for, it is not necessary to use C linkage to interpose.

It's possible that C linkage is needed for some reason, but this comment does not explain it. Ideally, we should be have complete understanding of why we are using particular language constructs, but at the very least, this comment should be removed.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:135
> +    typedef struct shmDescriptor {

This is C++ code, and "typedef struct" should not be used.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:136
> +        struct shmDescriptor *next;

Same concern about incorrect style. This issue is present many times below.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:185
> +        if (descriptorPtr->referenceCount == 0) {

More comparison to zero.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:215
> +            mappedAddress = descriptorPtr->mmapedAddress = mmap((void *)requestedSharedAddress,
> +                                                          descriptorPtr->mmapedSize,
> +                                                          PROT_READ | PROT_WRITE,
> +                                                          MAP_ANON | MAP_PRIVATE, -1, 0);

WebKit coding style forbids this this kind of formatting.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:237
> +        if (descriptorPtr == NULL) {

Comparison to zero.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:270
> +        case IPC_RMID:
> +                errno = EPERM;
> +                goto failed;

Please use 4-space indentation, not 8-space.

> Source/WebKit2/PluginProcess/mac/PluginProcessShim.mm:283
> +                outputDescriptor->shm_ctime = outputDescriptor->shm_atime = outputDescriptor->shm_dtime = time(NULL);

We use 0 or nullptr, not NULL.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.