Skip to content

Commit 625e105

Browse files
committed
Merge pull request #613 from agilob/codequalitymeasures
Improve code quality - fix security and performance issues, better error handling
2 parents cb93f6b + 7bb5e74 commit 625e105

28 files changed

+1119
-2252
lines changed

app/src/main/java/com/SecUpwN/AIMSICD/AIMSICD.java

+67-87
Large diffs are not rendered by default.

app/src/main/java/com/SecUpwN/AIMSICD/activities/DebugLogs.java

+22-23
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,9 @@
5959
*
6060
*/
6161

62-
6362
public class DebugLogs extends BaseActivity {
6463

65-
private static final String TAG = "AIMSICD";
66-
private static final String mTAG = "DebugLogs";
64+
private static final String TAG = "DebugLogs";
6765

6866
private LogUpdaterThread logUpdater = null;
6967
private boolean updateLogs = true;
@@ -73,7 +71,6 @@ public class DebugLogs extends BaseActivity {
7371
private Button btnClear = null;
7472
private Button btnCopy = null;
7573
private Button btnStop = null;
76-
//private Button btnRadio = null;
7774

7875
@Override
7976
public void onCreate(Bundle savedInstanceState) {
@@ -86,7 +83,6 @@ public void onCreate(Bundle savedInstanceState) {
8683
btnClear = (Button) findViewById(R.id.btnClear);
8784
btnStop = (Button) findViewById(R.id.btnStopLogs);
8885
btnCopy = (Button) findViewById(R.id.btnCopy);
89-
//btnRadio = (Button) findViewById(R.id.btnRadio);
9086

9187
runOnUiThread(new Runnable() {
9288
@Override
@@ -100,9 +96,8 @@ public void run() {
10096
public void onClick(View view) {
10197
try {
10298
clearLogs();
103-
//Log.d("DebugLogs", "Logcat clearing disabled!");
104-
} catch (Exception e) {
105-
Log.e(TAG, mTAG + ": Error clearing logs", e);
99+
} catch (IOException e) {
100+
Log.e(TAG, "Error clearing logs", e);
106101
}
107102
}
108103
});
@@ -161,12 +156,10 @@ protected void onResume() {
161156

162157
private void startLogging() {
163158
updateLogs = true;
164-
try {
165-
logUpdater = new LogUpdaterThread();
166-
logUpdater.start();
167-
} catch (Exception e) {
168-
Log.e(TAG, mTAG + ": Error starting log updater thread", e);
169-
}
159+
160+
logUpdater = new LogUpdaterThread();
161+
logUpdater.start();
162+
170163
btnStop.setText(getString(R.string.btn_stop_logs));
171164
}
172165

@@ -218,7 +211,7 @@ public void run() {
218211
intent.putExtra(Intent.EXTRA_TEXT, log);
219212
startActivity(Intent.createChooser(intent, "Send Error Log"));
220213
} catch (IOException e) {
221-
Log.e(TAG, mTAG + ": Error reading logs", e);
214+
Log.w(TAG, "Error reading logs", e);
222215
}
223216
}
224217
}.start();
@@ -280,8 +273,10 @@ private String runProcess(String command) throws IOException {
280273
*/
281274
private String runProcess(String[] command) throws IOException {
282275
Process process = null;
283-
if (command.length == 1) process = Runtime.getRuntime().exec(command[0]);
284-
else Runtime.getRuntime().exec(command);
276+
if (command.length == 1)
277+
process = Runtime.getRuntime().exec(command[0]);
278+
else
279+
Runtime.getRuntime().exec(command);
285280

286281
BufferedReader bufferedReader = new BufferedReader(
287282
new InputStreamReader(process.getInputStream()));
@@ -292,6 +287,7 @@ private String runProcess(String[] command) throws IOException {
292287
log.append(line);
293288
log.append("\n");
294289
}
290+
bufferedReader.close();
295291
return log.toString();
296292
}
297293

@@ -306,8 +302,8 @@ private void clearLogs() throws IOException {
306302
public void run() {
307303
try {
308304
Runtime.getRuntime().exec("logcat -c -b main -b system -b radio -b events");
309-
} catch (Exception e) {
310-
Log.e(TAG, mTAG + ": Error clearing logs", e);
305+
} catch (IOException e) {
306+
Log.e(TAG, "Error clearing logs", e);
311307
}
312308

313309
runOnUiThread(new Runnable() {
@@ -325,7 +321,6 @@ class LogUpdaterThread extends Thread {
325321
public void run() {
326322
while (updateLogs) {
327323
try {
328-
//Log.d("log_thread", "running");
329324
final String logs = getLogs();
330325
if (!logs.equals(logView.getText().toString())) {
331326
runOnUiThread(new Runnable() {
@@ -345,10 +340,14 @@ public void run() {
345340
}
346341
});
347342
}
348-
} catch (Exception e) {
349-
Log.e(TAG, mTAG + ": Error updating logs", e);
343+
} catch (IOException e) {
344+
Log.w(TAG, "Error updating logs", e);
345+
}
346+
try {
347+
Thread.sleep(1000);
348+
} catch (InterruptedException e) {
349+
Log.w(TAG, "Thread was interrupted", e);
350350
}
351-
try { Thread.sleep(1000); } catch (Exception e) {}
352351
}
353352
}
354353
}

app/src/main/java/com/SecUpwN/AIMSICD/activities/MapViewerOsmDroid.java

+62-59
Original file line numberDiff line numberDiff line change
@@ -263,13 +263,13 @@ private void setupMapType(int mapType) {
263263
// as they are redundant (hybrid) and broken (satellite).
264264
switch (mapType) {
265265
case 0:
266-
mMap.setTileSource(TileSourceFactory.DEFAULT_TILE_SOURCE); //setMapType(GoogleMap.MAP_TYPE_NORMAL);
266+
mMap.setTileSource(TileSourceFactory.DEFAULT_TILE_SOURCE);
267267
break;
268268
case 1:
269-
mMap.setTileSource(TileSourceFactory.CYCLEMAP); //.setMapType(GoogleMap.MAP_TYPE_TERRAIN);
269+
mMap.setTileSource(TileSourceFactory.CYCLEMAP);
270270
break;
271271
default:
272-
mMap.setTileSource(TileSourceFactory.DEFAULT_TILE_SOURCE); //setMapType(GoogleMap.MAP_TYPE_NORMAL);
272+
mMap.setTileSource(TileSourceFactory.DEFAULT_TILE_SOURCE);
273273
break;
274274
}
275275
}
@@ -392,11 +392,9 @@ private void loadEntries() {
392392
new AsyncTask<Void,Void,GeoPoint>() {
393393
@Override
394394
protected GeoPoint doInBackground(Void... voids) {
395-
final int SIGNAL_SIZE_RATIO = 15; // A scale factor to draw BTS Signal circles
396395
int signal;
397396

398397
mCellTowerGridMarkerClusterer.getItems().clear();
399-
// loadOpenCellIDMarkers();
400398

401399
//New function only gets bts from DBe_import by sim network
402400
loadOcidMarkersByNetwork();
@@ -407,9 +405,8 @@ protected GeoPoint doInBackground(Void... voids) {
407405
try {
408406
// Grab cell data from CELL_TABLE (cellinfo) --> DBi_bts
409407
c = mDbHelper.getCellData();
410-
411-
}catch(IllegalStateException ix) {
412-
Log.e(TAG, ix.getMessage(), ix);
408+
} catch(IllegalStateException ix) {
409+
Log.e(TAG, "Problem getting data from CELL_TABLE", ix);
413410
}
414411

415412
/*
@@ -425,32 +422,35 @@ protected GeoPoint doInBackground(Void... voids) {
425422
final int mnc = c.getInt(c.getColumnIndex(DBTableColumnIds.DBI_BTS_MNC)); // MNC
426423
final double dlat = c.getDouble(c.getColumnIndex(DBTableColumnIds.DBI_BTS_LAT)); // Lat
427424
final double dlng = c.getDouble(c.getColumnIndex(DBTableColumnIds.DBI_BTS_LON)); // Lon
428-
if (dlat == 0.0 && dlng == 0.0) {
425+
426+
if (Double.doubleToRawLongBits(dlat) == 0
427+
&& Double.doubleToRawLongBits(dlng) == 0) {
429428
continue;
430429
}
431-
//TODO this (signal) is not in DBi_bts
432-
signal = 1;//c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_AVG_SIGNAL)); // signal
430+
// TODO this (signal) is not in DBi_bts
431+
signal = 1;
432+
//c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_AVG_SIGNAL)); // signal
433433
// In case of missing or negative signal, set a default fake signal,
434434
// so that we can still draw signal circles. ?
435-
if (signal <= 0) {
436-
signal = 20;
437-
}
435+
//if (signal <= 0) {
436+
// signal = 20;
437+
//}
438438

439-
if ((dlat != 0.0) || (dlng != 0.0)) {
439+
if (Double.doubleToRawLongBits(dlat) != 0
440+
|| Double.doubleToRawLongBits(dlng) != 0) {
440441
loc = new GeoPoint(dlat, dlng);
441442

442-
443443
CellTowerMarker ovm = new CellTowerMarker(mContext, mMap,
444-
"Cell ID: " + cellID,
445-
"", loc,
446-
new MarkerData(
447-
"" + cellID,
448-
"" + loc.getLatitude(),
449-
"" + loc.getLongitude(),
450-
"" + lac,
451-
"" + mcc,
452-
"" + mnc,
453-
"", false)
444+
"Cell ID: " + cellID,
445+
"", loc,
446+
new MarkerData(
447+
String.valueOf(cellID),
448+
String.valueOf(loc.getLatitude()),
449+
String.valueOf(loc.getLongitude()),
450+
String.valueOf(lac),
451+
String.valueOf(mcc),
452+
String.valueOf(mnc),
453+
"", false)
454454
);
455455
// The pin of our current position
456456
ovm.setIcon(getResources().getDrawable(R.drawable.ic_map_pin_blue));
@@ -482,10 +482,14 @@ public void run() {
482482
c.close();
483483
}
484484
// plot neighbouring cells
485-
while (mAimsicdService == null) try {
486-
if (isCancelled()) return null;
487-
Thread.sleep(100);
488-
} catch (Exception e) {}
485+
while (mAimsicdService == null)
486+
try {
487+
if (isCancelled())
488+
return null;
489+
Thread.sleep(100);
490+
} catch (InterruptedException e) {
491+
Log.w(TAG, "thread interrupted", e);
492+
}
489493
List<Cell> nc = mAimsicdService.getCellTracker().updateNeighbouringCells();
490494
for (Cell cell : nc) {
491495
if (isCancelled()) return null;
@@ -495,13 +499,13 @@ public void run() {
495499
getString(R.string.cell_id_label) + cell.getCID(),
496500
"", loc,
497501
new MarkerData(
498-
"" + cell.getCID(),
499-
"" + loc.getLatitude(),
500-
"" + loc.getLongitude(),
501-
"" + cell.getLAC(),
502-
"" + cell.getMCC(),
503-
"" + cell.getMNC(),
504-
"", false));
502+
String.valueOf(cell.getCID()),
503+
String.valueOf(loc.getLatitude()),
504+
String.valueOf(loc.getLongitude()),
505+
String.valueOf(cell.getLAC()),
506+
String.valueOf(cell.getMCC()),
507+
String.valueOf(cell.getMNC()),
508+
"", false));
505509

506510
// The pin of other BTS
507511
ovm.setIcon(getResources().getDrawable(R.drawable.ic_map_pin_orange));
@@ -526,7 +530,8 @@ public void run() {
526530
*/
527531
@Override
528532
protected void onPostExecute(GeoPoint defaultLoc) {
529-
if (loc != null && (loc.getLatitude() != 0.0 && loc.getLongitude() != 0.0)) {
533+
if (loc != null && (Double.doubleToRawLongBits(loc.getLatitude()) != 0
534+
&& Double.doubleToRawLongBits(loc.getLongitude()) != 0)) {
530535
mMap.getController().setZoom(16);
531536
mMap.getController().animateTo(new GeoPoint(loc.getLatitude(), loc.getLongitude()));
532537
} else {
@@ -567,7 +572,7 @@ private void loadOcidMarkersByNetwork() {
567572
int imnc =0;
568573
if (networkOperator != null) {
569574
imcc = Integer.parseInt(networkOperator.substring(0, 3));
570-
imnc =Integer.parseInt(networkOperator.substring(3));
575+
imnc = Integer.parseInt(networkOperator.substring(3));
571576
}
572577
// DBe_import tower pins.
573578
Drawable cellTowerMarkerIcon = getResources().getDrawable(R.drawable.ic_map_pin_green);
@@ -577,36 +582,35 @@ private void loadOcidMarkersByNetwork() {
577582
if (c.moveToFirst()) {
578583
do {
579584
// CellID,Lac,Mcc,Mnc,Lat,Lng,AvgSigStr,Samples
580-
final int cellID = c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_CID)); // CellID
581-
final int lac = c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_LAC)); // Lac
582-
final int mcc = c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_MCC)); // Mcc
583-
final int mnc = c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_MNC)); // Mnc
584-
final double dlat = Double.parseDouble(c.getString(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_GPS_LAT))); // Lat
585-
final double dlng = Double.parseDouble(c.getString(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_GPS_LON))); // Lon
586-
final GeoPoint location = new GeoPoint(dlat, dlng); //
585+
final int cellID = c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_CID));
586+
final int lac = c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_LAC));
587+
final int mcc = c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_MCC));
588+
final int mnc = c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_MNC));
589+
final double dlat = Double.parseDouble(c.getString(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_GPS_LAT)));
590+
final double dlng = Double.parseDouble(c.getString(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_GPS_LON)));
591+
final GeoPoint location = new GeoPoint(dlat, dlng);
587592
//where is c.getString(6)AvgSigStr
588-
final int samples = c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_SAMPLES)); //Samples
593+
final int samples = c.getInt(c.getColumnIndex(DBTableColumnIds.DBE_IMPORT_SAMPLES));
589594
// Add map marker for CellID
590595
CellTowerMarker ovm = new CellTowerMarker(mContext, mMap,
591596
"Cell ID: " + cellID,
592597
"", location,
593598
new MarkerData(
594-
"" + cellID,
595-
"" + location.getLatitude(),
596-
"" + location.getLongitude(),
597-
"" + lac,
598-
"" + mcc,
599-
"" + mnc,
600-
"" + samples,
601-
false));
599+
String.valueOf(cellID),
600+
String.valueOf(location.getLatitude()),
601+
String.valueOf(location.getLongitude()),
602+
String.valueOf(lac),
603+
String.valueOf(mcc),
604+
String.valueOf(mnc),
605+
String.valueOf(samples),
606+
false));
602607

603608
ovm.setIcon(cellTowerMarkerIcon);
604609
items.add(ovm);
605610
} while (c.moveToNext());
606611
}
607612
c.close();
608613

609-
610614
mCellTowerGridMarkerClusterer.addAll(items);
611615
}
612616
public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, String key) {
@@ -619,8 +623,7 @@ public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, Strin
619623

620624
public void setRefreshActionButtonState(final boolean refreshing) {
621625
if (mOptionsMenu != null) {
622-
final MenuItem refreshItem = mOptionsMenu
623-
.findItem(R.id.get_opencellid);
626+
final MenuItem refreshItem = mOptionsMenu.findItem(R.id.get_opencellid);
624627
if (refreshItem != null) {
625628
if (refreshing) {
626629
refreshItem.setActionView(R.layout.actionbar_indeterminate_progress);
@@ -641,7 +644,7 @@ public void onStop() {
641644
public void onStart() {
642645
super.onStart();
643646
((AppAIMSICD) getApplication()).attach(this);
644-
if( TinyDB.getInstance().getBoolean(TinyDbKeys.FINISHED_LOAD_IN_MAP)) {
647+
if(TinyDB.getInstance().getBoolean(TinyDbKeys.FINISHED_LOAD_IN_MAP)) {
645648
setRefreshActionButtonState(false);
646649
}
647650
}

0 commit comments

Comments
 (0)