FYI, moving variable definitions to the top is a pre-C89 practice. As of C89/C90 they can occur at first use. IoTivity coding standards are to declare local variables as close to their first use as possible and within the scope where they are used.
https://wiki.iotivity.org/iotivity_c_coding_standards#best_practices_recommendations For initialization, having an explicit value prevents accidental use of uninitialized memory/values. In the case where a value is not used before it is set again a compiler is free to optimize that away. On the other hand if some variables are left uninitialized then there is a risk that random program behavior and/or crashes will occur. On 08/04/2015 01:01 AM, Hung Yu-Hsin wrote: > Hi Jaehong, > I've taken your comments and made those changes in the latest commit. Please > check it at https://gerrit.iotivity.org/gerrit/#/c/2099/. > > For the goto exit part, because I personally didn't use goto, I'm not sure > what's the best practice. > If so, we need to move ALL the variable definitions to the top of function > jobject jni_obj_socket = NULL; // do we need to assign NULL value? > jni_obj_socket = ... > ... > exit: (*env)->DeleteLocalRef(env, jni_obj_socket); return CA_STATUS_FAILED; > // no other return values in the future? > Also, I'm not sure if we pass a NULL to DeleteLocalRef. > > > What's your suggestion? > > > Best Regard, === Yu-Hsin Hung,Intern, MediatekGraduate Student, Institute of > Computer Science and Engineering,National Chiao-Tung Universiry, Hsinchu 300, > Taiwan (Phone) +886911012113(Email) hungys at hotmail.com (Website) > http://hungys.logdown.com/ === > > Date: Tue, 4 Aug 2015 05:53:34 +0000 > From: jaehong.jo at samsung.com > Subject: Re: [dev] [Android] JNI local reference table overflow (CA) > To: hungys at hotmail.com; iotivity-dev at lists.iotivity.org > > > > > > Hi, Yu-Hsin. > > I have confirmed your modifications, seems almost good. > Please check my (few) comments. > And I have added maintainer for reviewer. > Once confirmed, your changes will be merged. > Thank you for your contribution. > > Regards, > Jaehong. > > ------- Original Message ------- > > Sender : Hung Yu-Hsin<hungys at hotmail.com> > > Date : 2015-08-04 12:09 (GMT+09:00) > > Title : RE: [dev] [Android] JNI local reference table overflow (CA) > > > > > > Hi Jaehong, > > > I've commited the changes, please review it. > https://gerrit.iotivity.org/gerrit/#/c/2099/ > > > This version has been tested to run for 90 minutes without any crash (client > keeps sending GET request every 2 sec) > > > > > Best Regard, === Yu-Hsin Hung, > Intern, Mediatek > Graduate Student, Institute of Computer Science and Engineering, > National Chiao-Tung Universiry, Hsinchu 300, Taiwan > (Phone) +886911012113 > (Email) hungys at hotmail.com (Website) http://hungys.logdown.com/ === > > > > > Date: Mon, 3 Aug 2015 11:41:50 +0000 > From: jaehong.jo at samsung.com > Subject: RE: [dev] [Android] JNI local reference table overflow (CA) > To: hungys at hotmail.com; iotivity-dev at lists.iotivity.org > > > Hi, Yu-Hsin. > Sounds great. > Add me as your reviewer to confirm. > Have a nice day. > > Regards, > Jaehong. > ------- Original Message ------- > Sender : Hung Yu-Hsin<hungys at hotmail.com> > Date : 2015-08-03 17:35 (GMT+09:00) > Title : RE: [dev] [Android] JNI local reference table overflow (CA) > > > > > Hi Jaehong, > > > I've seen your patch this morning, but I still found some memory leak issues > that cause application crashed after few minutes due to JNI local reference > table overflow. (My client just keeps polling to server every 5 seconds) > > > I think I've found where the memory leak occurs although I'm not sure why > they're not handled by JVM. Based on your patch, I add few lines in EDR > implementation to prevent memory leak and now it has been tested to run for > thirty minutes without crash. Once I confirm the patch does fix the issue, I > can try to submit it. > > > Thank you! > > > > > > Best Regard, === Yu-Hsin Hung, > Electrical Engineering and Computer Science Undergraduate Honors Program, > National Chiao-Tung Universiry, Hsinchu 300, Taiwan > (Phone) +886911012113 > (Email) hungys at hotmail.com (Website) http://hungys.logdown.com/ === > > > > > Date: Mon, 3 Aug 2015 08:25:49 +0000 > From: jaehong.jo at samsung.com > Subject: Re: [dev] [Android] JNI local reference table overflow (CA) > To: hungys at hotmail.com; iotivity-dev at lists.iotivity.org > > > Hi, Yu-Hsin. > > Recently we found an issue in CA with BT transport where InputStream is > created for each BT read. > So I had fixed that problem and submitted patch @ > https://gerrit.iotivity.org/gerrit/#/c/2017/ > > With above fix, we are able to transmit the data between devices using CA > sample as BT support is not yet integrated with RI and samples. > > Please check the latest version. If you have the same problem, log a jira > ticket with debug log. > (Scons build using option RELEASE = 0) @ http://jira.iotivity.org > > > Regards, > Jaehong. > > > ------- Original Message ------- > Sender : Hung Yu-Hsin<hungys at hotmail.com> > Date : 2015-07-30 18:29 (GMT+09:00) > Title : [dev] [Android] JNI local reference table overflow (CA) > > > > > Hi All, > > > Recently I found that the multi transport restriction of Android in CA's > scons script is removed, so I tried to build a new aar to support IP/EDR/LE > simultaneously. > > > My application runs well before I added reference to the new aar, although it > only support IP adapter due to IoTivity limitation. After integrate with the > new aar, the Android sample app: simpleclient and simpleserver, work great > with Bluetooth EDR connection, but often crashed on my application. > > > Basically my client will do polling to server periodically (assume 5s), and > the app always crashed after about one minute. The log says there is an > error: "JNI ERROR (app bug): local reference table overflow (max=512)", so I > guess there are some issues in the CA implementation. > > > Also, if I set the period more frequently, almost no operation will succeed > (findResource, get...), I suspect the implementation of EDR is not > thread-safe, so the data operation on the Bluetooth interface will lead to > unexpected behavior? > > > Here are some logs of my application for this issue: > http://pastie.org/10319711 > > > Thanks! > > > > Best Regard, === Yu-Hsin Hung, > Electrical Engineering and Computer Science Undergraduate Honors Program, > National Chiao-Tung Universiry, Hsinchu 300, Taiwan > (Phone) +886911012113 > (Email) hungys at hotmail.com (Website) http://hungys.logdown.com/ === > > > > > > > > > _______________________________________________ > iotivity-dev mailing list > iotivity-dev at lists.iotivity.org > https://lists.iotivity.org/mailman/listinfo/iotivity-dev > -- Jon A. Cruz - Senior Open Source Developer Samsung Open Source Group jonc at osg.samsung.com
