1374699 - Compiler warnings for nICEr and nrappkit

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, enhancement, P3)

Description

[Michael Froman [:mjf]]

Here are the compiler warnings I see on OS X builds with clang 8.1.0:
5:27.84 warning: media/mtransport/third_party/nICEr/src/ice/ice_component.c:998:5 [-Wmissing-prototypes] no previous prototype for function 'nr_ice_component_can_candidate_tcptype_pair'
5:27.84 warning: media/mtransport/third_party/nICEr/src/ice/ice_component.c:1015:5 [-Wmissing-prototypes] no previous prototype for function 'nr_ice_component_can_candidate_addr_pair'
5:27.84 warning: media/mtransport/third_party/nICEr/src/ice/ice_component.c:1152:5 [-Wmissing-prototypes] no previous prototype for function 'nr_ice_pre_answer_enqueue'
5:27.84 warning: media/mtransport/third_party/nICEr/src/ice/ice_component.c:1326:5 [-Wmissing-prototypes] no previous prototype for function 'nr_ice_component_refresh_consent'
5:27.84 warning: media/mtransport/third_party/nICEr/src/ice/ice_component.c:1340:6 [-Wmissing-prototypes] no previous prototype for function 'nr_ice_component_consent_calc_consent_timer'
5:27.84 warning: media/mtransport/third_party/nICEr/src/ice/ice_component.c:1433:5 [-Wmissing-prototypes] no previous prototype for function 'nr_ice_component_setup_consent'
5:27.84 warning: media/mtransport/third_party/nICEr/src/ice/ice_ctx.c:919:14 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned long'
5:27.84 warning: media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c:730:5 [-Wimplicit-function-declaration] implicit declaration of function 'nr_ice_component_consent_destroy' is invalid in C99
5:27.84 warning: media/mtransport/third_party/nICEr/src/net/transport_addr.c:478:26 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned long'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_build.c:328:53 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_client_ctx.c:246:25 [-Wsign-compare] comparison of integers of different signs: 'int' and 'UINT4' (aka 'unsigned int')
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_client_ctx.c:256:5 [-Wimplicit-function-declaration] implicit declaration of function 'nr_ice_accumulate_count' is invalid in C99
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_client_ctx.c:282:25 [-Wsign-compare] comparison of integers of different signs: 'int' and 'UINT4' (aka 'unsigned int')
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_client_ctx.c:405:29 [-Wsign-compare] comparison of integers of different signs: 'int' and 'UINT4' (aka 'unsigned int')
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:92:28 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:108:28 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:124:28 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:155:28 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:172:28 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:189:28 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:231:25 [-Wsign-compare] comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:564:17 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned long'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:640:21 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned long'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:971:24 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:974:28 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:1220:19 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned long'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:1393:40 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_codec.c:1467:39 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned long'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_hint.c:70:39 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.84 warning: media/mtransport/third_party/nICEr/src/stun/stun_hint.c:170:39 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.85 warning: media/mtransport/third_party/nICEr/src/stun/stun_hint.c:187:39 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.85 warning: media/mtransport/third_party/nICEr/src/stun/stun_hint.c:204:39 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.85 warning: media/mtransport/third_party/nICEr/src/stun/stun_hint.c:224:39 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.85 warning: media/mtransport/third_party/nICEr/src/stun/stun_msg.c:77:16 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned long'
5:27.85 warning: media/mtransport/third_party/nICEr/src/stun/stun_msg.c:228:56 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.85 warning: media/mtransport/third_party/nICEr/src/stun/stun_util.c:97:29 [-Wsign-compare] comparison of integers of different signs: 'int' and 'unsigned long'
5:27.85 warning: media/mtransport/third_party/nICEr/src/stun/stun_util.c:217:24 [-Wsign-compare] comparison of integers of different signs: 'unsigned long' and 'int'
5:27.85 warning: media/mtransport/third_party/nICEr/src/stun/turn_client_ctx.c:260:9 [-Wimplicit-function-declaration] implicit declaration of function 'nr_ice_accumulate_count' is invalid in C99
5:27.85 warning: media/mtransport/third_party/nICEr/src/stun/turn_client_ctx.c:905:15 [-Wsign-compare] comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int'

Comment 1

[Michael Froman [:mjf]]

Attachment: Bug 1374699 - fixing compiler warnings for nICEr.

Cleaning up compiler warnings for nICEr.  Major highlights:
 - set _WINSOCK_DEPRECATED_NO_WARNINGS define in nicer.gyp for Windows
   builds of nICEr to avoid warnings about inet_addr use in ice_ctx.c:102,
   ice_ctx.c:297, ice_parser.c:465, and transport_addr_reg.c:143.
 - move nr_ice_accumulate_count from ice_ctx.{h|c} to stun_util.{h|c}
   as nr_accumulate_count to quiet warnings in turn_client_ctx.c and
   stun_client_ctx.c.
 - stun_msg.{h|c} - change nr_stun_attr_data_.length,
   nr_stun_message_attribute.encoding_length and nr_stun_message_.length
   from int to UINT2 (not size_t since other lengths in this header are
   UINT2).
 - stun_codec.{h|c} - lengths and offsets changed from int to UINT2 to
   match changes in stun_msg.{h|c}
 - r_data.{h|c} - change Data.len from int to size_t
 - nr_crypto.{h|c} - change nr_ice_crypto_vtbl_ lengths from int to size_t

Review commit: https://reviewboard.mozilla.org/r/158044/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158044/

Comment 2

[Nils Ohlmeier [:drno]]

Friendly reminder to review or postpone :-)

Comment 3

[Bulk Bug Changes for mreavy's org]

Mass change P2->P3 to align with new Mozilla triage process.

Comment 4

[Michael Froman [:mjf]]

Attachment: Bug 1374699 - fixing compiler warnings for nrappkit.

Review commit: https://reviewboard.mozilla.org/r/183264/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183264/

Comment 5

[Michael Froman [:mjf]]

Attachment: Bug 1374699 - fix .gyp file formatting by detabbing.

Review commit: https://reviewboard.mozilla.org/r/183266/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183266/

Comment 6

[Michael Froman [:mjf]]

Attachment: Bug 1374699 - make warnings errors on nICEr and nrappkit builds.

Review commit: https://reviewboard.mozilla.org/r/183268/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183268/

Comment 7

[Michael Froman [:mjf]]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158044/diff/1-2/

Comment 8

[Eric Rescorla (:ekr)]

Comment on attachment 8911893 [details]
Bug 1374699 - fixing compiler warnings for nrappkit.

https://reviewboard.mozilla.org/r/183264/#review195528

::: media/mtransport/third_party/nrappkit/src/log/r_log.c:199
(Diff revision 1)
>        log_types[i].level[j]=LOG_LEVEL_UNDEFINED;
>  
>        if(NR_reg_initted()){
>  
>          if(snprintf(dest_prefix,sizeof(NR_registry),
> -          "logging.%s.facility",log_destinations[j].dest_name)>=sizeof(NR_registry))
> +          "logging.%s.facility",log_destinations[j].dest_name)>=(int)sizeof(NR_registry))

For all these, you need to upcast to size_t not downcast to int

::: media/mtransport/third_party/nrappkit/src/log/r_log.c
(Diff revision 1)
>  
>      *lt_level=level;
> -
> -    _status=0;
> -  abort:
> -    return;

Please do not make this change. We deliberately use ABORT(), etc. to make room for cleanup code and because you can log on ABORT()

::: media/mtransport/third_party/nrappkit/src/log/r_log.c:564
(Diff revision 1)
>  
>        /* Get the data out of the registry */
>        for(i=0; i<LOG_NUM_DESTINATIONS; i++){
>          /* set callback for default level */
>          if(snprintf(reg_key,sizeof(reg_key),"%s.%s.level",LOGGING_REG_PREFIX,
> -          log_destinations[i].dest_name)>=sizeof(reg_key))
> +          log_destinations[i].dest_name)>=(int)sizeof(reg_key))

You should upcast to size_t, not downcast to int, here and below.

::: media/mtransport/third_party/nrappkit/src/registry/registry_local.c:997
(Diff revision 1)
>  NRREGSET(nr_reg_local_set_uint4,    NR_REG_TYPE_UINT4,    UINT4)
>  NRREGSET(nr_reg_local_set_int8,     NR_REG_TYPE_INT8,     INT8)
>  NRREGSET(nr_reg_local_set_uint8,    NR_REG_TYPE_UINT8,    UINT8)
>  NRREGSET(nr_reg_local_set_double,   NR_REG_TYPE_DOUBLE,   double)
>  
> +#undef NRREGSET // undef so it doesn't collide with registry.c version

What's going on here? these are separate source files. See also above.

::: tools/rewriting/ThirdPartyPaths.txt
(Diff revision 1)
>  media/libtheora/
>  media/libtremor/
>  media/libvorbis/
>  media/libvpx/
>  media/libyuv/
> -media/mtransport/third_party/nrappkit

What does this change do?

Comment 9

[Eric Rescorla (:ekr)]

Comment on attachment 8911894 [details]
Bug 1374699 - fix .gyp file formatting by detabbing.

https://reviewboard.mozilla.org/r/183266/#review195534

Comment 10

[Eric Rescorla (:ekr)]

Comment on attachment 8911895 [details]
Bug 1374699 - make warnings errors on nICEr and nrappkit builds.

https://reviewboard.mozilla.org/r/183268/#review195536

::: media/mtransport/third_party/nrappkit/nrappkit.gyp:186
(Diff revision 1)
>  
>                   'sources': [
>                        './src/port/darwin/include/csi_platform.h',
>                   ],
>                }],
>                

Maybe fix this while you're in here.

Comment 11

[Eric Rescorla (:ekr)]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

https://reviewboard.mozilla.org/r/158044/#review195538

I see a lot of unnecessary name changes here. Please revert those so that this is easier to review, and then I will take a look.

::: media/mtransport/third_party/nrappkit/src/util/libekr/r_data.c:93
(Diff revision 2)
>  #include <string.h>
>  
> -int r_data_create(dp,d,l)
> +int r_data_create(dp,d,len)
>    Data **dp;
>    const UCHAR *d;
> -  int l;
> +  size_t len;

Please don't just wantonly rename stuff. The convention in this code is for short names and it makes reading this diff much harder.

::: tools/rewriting/ThirdPartyPaths.txt:43
(Diff revision 2)
>  media/libtheora/
>  media/libtremor/
>  media/libvorbis/
>  media/libvpx/
>  media/libyuv/
> -media/mtransport/third_party/
> +media/mtransport/third_party/nrappkit

What does this do?

Comment 12

[Michael Froman [:mjf]]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

https://reviewboard.mozilla.org/r/158044/#review195538

> Please don't just wantonly rename stuff. The convention in this code is for short names and it makes reading this diff much harder.

Sorry to double up a readablity fix with the compiler warnings fix.  I'll revert the name changes.

> What does this do?

Normally during mach builds, warnings are spit out mid-stream and then again at the end of the build process.  The ThirdPartyPaths.txt file causes the second set of warnings emitted at the end of the build to be rolled up into a line that looks like:
 2:57.61 (suppressed 63 warnings in media/mtransport/third_party)
 
In this case, I'm ending the rollup for media/mtransport/third_party/nICEr, but leaving it for nrappkit since I haven't fixed those warnings in this patch.

Comment 13

[Michael Froman [:mjf]]

Comment on attachment 8911893 [details]
Bug 1374699 - fixing compiler warnings for nrappkit.

https://reviewboard.mozilla.org/r/183264/#review195528

> What's going on here? these are separate source files. See also above.

This is a side effect of unified builds.  I can either leave the undefs in registry.c and registry_local.c or I can add these two files to the list of non-unified sources in media/mtransport/third_party/moz.build.  Please let me know which option you'd prefer.

> What does this change do?

More complete explanation in the nICEr review, but here I'm ending the warnings rollup for all of media/mtransport/third_party now that I've fixed the warnings in nrappkit.

Comment 14

[Michael Froman [:mjf]]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158044/diff/2-3/

Comment 15

[Michael Froman [:mjf]]

Comment on attachment 8911893 [details]
Bug 1374699 - fixing compiler warnings for nrappkit.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183264/diff/1-2/

Comment 16

[Michael Froman [:mjf]]

Comment on attachment 8911894 [details]
Bug 1374699 - fix .gyp file formatting by detabbing.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183266/diff/1-2/

Comment 17

[Michael Froman [:mjf]]

Comment on attachment 8911895 [details]
Bug 1374699 - make warnings errors on nICEr and nrappkit builds.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183268/diff/1-2/

Comment 18

[Eric Rescorla (:ekr)]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

https://reviewboard.mozilla.org/r/158044/#review233590

Closer, but still some iseuss.

::: media/mtransport/nricectx.cpp:739
(Diff revision 3)
>  void NrIceCtx::AccumulateStats(const NrIceStats& stats) {
> -  nr_ice_accumulate_count(&(ctx_->stats.stun_retransmits),
> +  nr_accumulate_count(&(ctx_->stats.stun_retransmits),
> -                          stats.stun_retransmits);
> +                      stats.stun_retransmits);
> -  nr_ice_accumulate_count(&(ctx_->stats.turn_401s), stats.turn_401s);
> -  nr_ice_accumulate_count(&(ctx_->stats.turn_403s), stats.turn_403s);
> -  nr_ice_accumulate_count(&(ctx_->stats.turn_438s), stats.turn_438s);
> +  nr_accumulate_count(&(ctx_->stats.turn_401s), stats.turn_401s);
> +  nr_accumulate_count(&(ctx_->stats.turn_403s), stats.turn_403s);
> +  nr_accumulate_count(&(ctx_->stats.turn_438s), stats.turn_438s);



::: media/mtransport/third_party/nICEr/src/ice/ice_component.c:822
(Diff revision 3)
>        ABORT(r);
>      }
>  
>      _status=0;
>    abort:
> -    return(r);
> +    return(_status);

Good

::: media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
(Diff revision 3)
> -      // don't rollover, just stop accumulating at MAX value
> -      *orig_count = UINT2_MAX;
> -    } else {
> -      *orig_count += new_count;
> -    }
> -  }

What happened here?

::: media/mtransport/third_party/nICEr/src/stun/stun_codec.h:46
(Diff revision 3)
>  
>  typedef struct nr_stun_attr_codec_ {
>      char     *name;
>      int     (*print)(nr_stun_attr_info *attr_info, char *msg, void *data);
> -    int     (*encode)(nr_stun_attr_info *attr_info, void *data, int offset, int buflen, UCHAR *buf, int *attrlen);
> -    int     (*decode)(nr_stun_attr_info *attr_info, int attrlen, UCHAR *buf, int offset, int buflen, void *data);
> +    int     (*encode)(nr_stun_attr_info *attr_info, void *data, UINT2 offset, UINT2 buflen, UCHAR *buf, UINT2 *attrlen);
> +    int     (*decode)(nr_stun_attr_info *attr_info, UINT2 attrlen, UCHAR *buf, UINT2 offset, UINT2 buflen, void *data);

I don't think you should be using UINT2 for lengths, even of things which cannot be > 2^16-1. That's what size_t is for

::: media/mtransport/third_party/nICEr/src/stun/stun_msg.c:241
(Diff revision 3)
>  
>  int
>  nr_stun_message_add_nonce_attribute(nr_stun_message *msg, char *nonce)
>  NR_STUN_MESSAGE_ADD_ATTRIBUTE(
>      NR_STUN_ATTR_NONCE,
> -    { strlcpy(attr->u.nonce, nonce, sizeof(attr->u.nonce)); }
> +    // prepend (void) to ignore return value

I don't thinkt his comment is necessary here or elsewhere

::: tools/rewriting/ThirdPartyPaths.txt:43
(Diff revision 3)
>  media/libtheora/
>  media/libtremor/
>  media/libvorbis/
>  media/libvpx/
>  media/libyuv/
> -media/mtransport/third_party/
> +media/mtransport/third_party/nrappkit

What is going on here? You shouldn't be rewriting nICEr either.

Comment 19

[Eric Rescorla (:ekr)]

Comment on attachment 8911893 [details]
Bug 1374699 - fixing compiler warnings for nrappkit.

https://reviewboard.mozilla.org/r/183264/#review195528

> This is a side effect of unified builds.  I can either leave the undefs in registry.c and registry_local.c or I can add these two files to the list of non-unified sources in media/mtransport/third_party/moz.build.  Please let me know which option you'd prefer.

I'd prefer these be non-unified builds. This code was designed to one file per translation unit.

> More complete explanation in the nICEr review, but here I'm ending the warnings rollup for all of media/mtransport/third_party now that I've fixed the warnings in nrappkit.

OK. What I want to make sure is that this code isn't reformatted in some sort of mass rewrite pass. Isn't this file used for that as well?

Comment 20

[Eric Rescorla (:ekr)]

Comment on attachment 8911893 [details]
Bug 1374699 - fixing compiler warnings for nrappkit.

https://reviewboard.mozilla.org/r/183264/#review195528

OK, so r+ once you 

1. de-unify these files 
2. Verify that you're not leaving things open to restyling

I can't figure out how to do that with Mozreview, but here it is

Comment 21

[Michael Froman [:mjf]]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

https://reviewboard.mozilla.org/r/158044/#review233590

> What happened here?

This function was moved to stun_util.{h|c} because it was going to be used in more than just nICEr/src/ice/*

> I don't think you should be using UINT2 for lengths, even of things which cannot be > 2^16-1. That's what size_t is for

I used UINT2 here because I was concerned that the size of size_t is platform dependent.  If you are comfortable with that, I'll make that change.

> What is going on here? You shouldn't be rewriting nICEr either.

Ah - I wasn't aware of that part of this file's usage.  I'll revert this change (and for nrappkit).

Comment 22

[Eric Rescorla (:ekr)]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

https://reviewboard.mozilla.org/r/158044/#review233590

> I used UINT2 here because I was concerned that the size of size_t is platform dependent.  If you are comfortable with that, I'll make that change.

Why is it a problem if it is platform dependent

Comment 23

[Michael Froman [:mjf]]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

https://reviewboard.mozilla.org/r/158044/#review233590

> Why is it a problem if it is platform dependent

I was concerned that might cause interop issues.  If not, then size_t is definitely better.

Comment 24

[Michael Froman [:mjf]]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

https://reviewboard.mozilla.org/r/158044/#review233590

> I was concerned that might cause interop issues.  If not, then size_t is definitely better.

I suppose after consideration, int could also be platform dependent, so there isn't much worry there.

Comment 25

[Michael Froman [:mjf]]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158044/diff/3-4/

Comment 26

[Michael Froman [:mjf]]

Comment on attachment 8911893 [details]
Bug 1374699 - fixing compiler warnings for nrappkit.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183264/diff/2-3/

Comment 27

[Michael Froman [:mjf]]

Comment on attachment 8911894 [details]
Bug 1374699 - fix .gyp file formatting by detabbing.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183266/diff/2-3/

Comment 28

[Michael Froman [:mjf]]

Comment on attachment 8911895 [details]
Bug 1374699 - make warnings errors on nICEr and nrappkit builds.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183268/diff/2-3/

Comment 29

[Eric Rescorla (:ekr)]

Michael, I think we're done here.

Comment 30

[Nils Ohlmeier [:drno]]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

https://reviewboard.mozilla.org/r/158044/#review245456

Since Ekr is done, I'll give the final LGTM

Comment 31

[Nils Ohlmeier [:drno]]

Comment on attachment 8911893 [details]
Bug 1374699 - fixing compiler warnings for nrappkit.

https://reviewboard.mozilla.org/r/183264/#review245458

Since Ekr is done, I'll give the final LGTM

Comment 32

[Michael Froman [:mjf]]

Comment on attachment 8887243 [details]
Bug 1374699 - fixing compiler warnings for nICEr.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/158044/diff/4-5/

Comment 33

[Michael Froman [:mjf]]

Comment on attachment 8911893 [details]
Bug 1374699 - fixing compiler warnings for nrappkit.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183264/diff/3-4/

Comment 34

[Michael Froman [:mjf]]

Comment on attachment 8911894 [details]
Bug 1374699 - fix .gyp file formatting by detabbing.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183266/diff/3-4/

Comment 35

[Michael Froman [:mjf]]

Comment on attachment 8911895 [details]
Bug 1374699 - make warnings errors on nICEr and nrappkit builds.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183268/diff/3-4/

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/92040641407c
fixing compiler warnings for nICEr. r=drno
https://hg.mozilla.org/integration/autoland/rev/d54c0f9794a4
fixing compiler warnings for nrappkit. r=drno
https://hg.mozilla.org/integration/autoland/rev/f99813328ce6
fix .gyp file formatting by detabbing. r=ekr
https://hg.mozilla.org/integration/autoland/rev/b32423756cb7
make warnings errors on nICEr and nrappkit builds. r=ekr

Comment 37

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/92040641407c
https://hg.mozilla.org/mozilla-central/rev/d54c0f9794a4
https://hg.mozilla.org/mozilla-central/rev/f99813328ce6
https://hg.mozilla.org/mozilla-central/rev/b32423756cb7