Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(114)

Issue 11299315: Cleanup: Fix various coding style issues in src/untrusted/pthread (Closed)

Created:
8 years ago by Mark Seaborn
Modified:
8 years ago
Reviewers:
Roland McGrath
CC:
native-client-reviews_googlegroups.com
Visibility:
Public.

Description

Cleanup: Fix various coding style issues in src/untrusted/pthread This includes: * Fix comment style * Fix "*" spacing * Remove spaces before arguments (e.g. "foo ()") * Add punctuation to comments * Remove "__" prefixes from local variables BUG=none TEST=build Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=10380

Patch Set 1 #

Total comments: 6

Patch Set 2 : Review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+161 lines, -144 lines) Patch
M src/untrusted/pthread/nc_condvar.c View 4 chunks +8 lines, -8 lines 0 comments Download
M src/untrusted/pthread/nc_mutex.c View 1 7 chunks +28 lines, -26 lines 0 comments Download
M src/untrusted/pthread/nc_thread.c View 1 35 chunks +119 lines, -104 lines 0 comments Download
M src/untrusted/pthread/pthread.h View 1 chunk +1 line, -1 line 0 comments Download
M src/untrusted/pthread/pthread_types.h View 1 3 chunks +5 lines, -5 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Mark Seaborn
8 years ago (2012-12-03 23:54:38 UTC) #1
Roland McGrath
LGTM. Additional cosmetic cleanups below are optional (and I didn't cite every case of the ...
8 years ago (2012-12-04 00:04:20 UTC) #2
Mark Seaborn
8 years ago (2012-12-05 05:10:57 UTC) #3
https://codereview.chromium.org/11299315/diff/1/src/untrusted/pthread/nc_mutex.c
File src/untrusted/pthread/nc_mutex.c (right):

https://codereview.chromium.org/11299315/diff/1/src/untrusted/pthread/nc_mute...
src/untrusted/pthread/nc_mutex.c:144: int pthread_once(pthread_once_t
*__once_control,
On 2012/12/04 00:04:20, Roland McGrath wrote:
> It's also a style violation (or should be) that this is using __name for local
> variables.

Done.

https://codereview.chromium.org/11299315/diff/1/src/untrusted/pthread/nc_thre...
File src/untrusted/pthread/nc_thread.c (right):

https://codereview.chromium.org/11299315/diff/1/src/untrusted/pthread/nc_thre...
src/untrusted/pthread/nc_thread.c:36: #define FUN_TO_VOID_PTR(a) ((void *)
((uintptr_t) a))
On 2012/12/04 00:04:20, Roland McGrath wrote:
> This also has superfluous parens around the second cast, but no parens around
> the macro argument, where they could (depending on the usage) be needed.  i.e.
> clean style would be: ((void *) (uintptr_t) (a))

Done.

https://codereview.chromium.org/11299315/diff/1/src/untrusted/pthread/nc_thre...
src/untrusted/pthread/nc_thread.c:114: /* If the function returns, terminate the
thread */
On 2012/12/04 00:04:20, Roland McGrath wrote:
> If you're going to capitalize, you might as well punctuate.

Done.

Powered by Google App Engine
This is Rietveld 408576698