self-diagnostics: fix datamodel access

self-diagnostics/Makefile

@@ -1,7 +1,7 @@
 include $(TOPDIR)/rules.mk
 
 PKG_NAME:=self-diagnostics
-PKG_VERSION:=1.0.16
+PKG_VERSION:=1.0.17
 PKG_RELEASE:=1
 
 PKG_LICENSE:=GPL-2.0-only

self-diagnostics/files/usr/libexec/rpcd/self-diagnostics

@@ -1,39 +0,0 @@
-#!/bin/sh
-
-BIN="/usr/sbin/self-diagnostics"
-. /usr/share/libubox/jshn.sh
-
-case "$1" in
-	list)
-		echo '{"list": {}, "generate" : {"modules":"String"}}'
-	;;
-	call)
-		case "$2" in
-			generate)
-				read -t 1 -r input
-				local out
-
-				json_load "${input}"
-				json_get_var modules modules
-				if [ -z "${modules}" ]; then
-					out="$(${BIN} -j)"
-				else
-					out="$(${BIN} -j -m "${modules}")"
-				fi
-				if [ -z "${out}" ]; then
-					echo '{}'
-				else
-					echo "${out}"
-				fi
-			;;
-			list)
-				out="$(${BIN} -j -l)"
-				if [ -z "${out}" ]; then
-					echo '{}'
-				else
-					echo "${out}"
-				fi
-			;;
-		esac
-	;;
-esac

self-diagnostics/files/usr/sbin/self-diagnostics

@@ -1,11 +1,12 @@
 #!/bin/sh
 
+# shellcheck disable=SC1091
 . /usr/share/libubox/jshn.sh
 
 JSON_OUT=0
 SPEC_DIR="/usr/share/self-diagnostics/spec"
 SPEC_EXT_DIR="/etc/self-diagnostics/spec"
-REPORT_PATH="/var/log/"
+REPORT_PATH="/var/log"
 REPORT_TEMP_DIR="$(mktemp -p ${REPORT_PATH} -d)"
 REPORT_NAME="self-test-diagnostics"
 VERBOSE=0
@@ -17,18 +18,19 @@ log()
 {
 	log_file="${REPORT_TEMP_DIR}/execution.log"
 	if [ "$VERBOSE" -eq 1 ]; then
-		logger -p debug -t $0 "$*"
+		logger -p debug -t "$0" "$*"
 	fi
-	echo "[$(date +%Y:%m:%d-%H:%M:%S)] $*" >> ${log_file}
+	echo "[$(date +%Y:%m:%d-%H:%M:%S)] $*" >> "${log_file}"
 }
 
 err_log()
 {
 	log_file="${REPORT_TEMP_DIR}/execution.log"
-	logger -p err -t $0 "$*"
+	logger -p err -t "$0" "$*"
-	echo "[$(date +%Y:%m:%d-%H:%M:%S) ERR] $*" >> ${log_file}
+	echo "[$(date +%Y:%m:%d-%H:%M:%S) ERR] $*" >> "${log_file}"
 }
 
+# shellcheck disable=SC3043
 generate_report()
 {
 	local filename
@@ -39,19 +41,19 @@ generate_report()
 	[ -f "${filename}.tar.gz" ] && rm "${filename}.tar.gz"
 
 	log "# Report generation completed #"
-	cd ${REPORT_TEMP_DIR} && {
+	cd "${REPORT_TEMP_DIR}" && {
 		filename="${filename}.tar"
-		tar -cf "${filename}" *
+		tar -cf "${filename}" ./*.log
 	}
 
 	if [ -n "$COMPOPTS" ]; then
-		gzip -${COMPOPTS} -f "${filename}"
+		gzip -"${COMPOPTS}" -f "${filename}"
 		filename="${filename}.gz"
 	fi
 
 	# Move logs if failed to generate tar
 	if [ ! -f "${filename}" ]; then
-		mv ${REPORT_TEMP_DIR}/*.log ${REPORT_PATH}/
+		mv "${REPORT_TEMP_DIR}"/*.log "${REPORT_PATH}"/
 	fi
 
 	if [ "${JSON_OUT}" -eq 1 ]; then
@@ -78,7 +80,7 @@ cleanup()
 {
 	if [ -d "${REPORT_TEMP_DIR}" ]; then
 		generate_report
-		rm -rf ${REPORT_TEMP_DIR}
+		rm -rf "${REPORT_TEMP_DIR}"
 	fi
 }
 
@@ -87,7 +89,7 @@ term_cleanup()
 	if [ -d "${REPORT_TEMP_DIR}" ]; then
 		err_log "Exiting due to TERM/INT signal"
 		generate_report
-		rm -rf ${REPORT_TEMP_DIR}
+		rm -rf "${REPORT_TEMP_DIR}"
 	fi
 }
 
@@ -111,8 +113,8 @@ help()
 # Alias ubus to have a smaller 5-second timeout on all subsequent calls
 ubus()
 {
-	if [ "${1}" == "call" ]; then
+	if [ "${1}" = "call" ]; then
-		if command ubus list $2 >/dev/null 2>&1; then
+		if command ubus list >/dev/null 2>&1; then
 			command ubus "$@";
 		fi
 	else
@@ -121,6 +123,7 @@ ubus()
 
 }
 
+# shellcheck disable=SC3043,SC3060,SC2034
 config_load()
 {
 	local temp
@@ -157,7 +160,7 @@ config_load()
 
 	temp="$(uci -q get self-diagnostics.globals.report_name)"
 	[ -n "${temp}" ] && \
-		REPORT_NAME="$(eval echo ${temp})"
+		REPORT_NAME="$(eval echo "${temp}")"
 
 	REPORT_NAME="${REPORT_NAME//[ \/]/_}"
 
@@ -170,9 +173,42 @@ config_load()
 		VERBOSE="${temp}"
 }
 
+# shellcheck disable=SC2129,SC3043
+run_cmd()
+{
+	local exec_timeout name cmd description
+	local export_path rc start_time end_time
+
+	exec_timeout="${1}"; shift
+	name="${1}"; shift
+	cmd="${1}"; shift
+	description="${*}"
+
+	start_time="$(date +%s)"
+	export_path="${REPORT_TEMP_DIR}/${name}.log"
+	log "Executing $cmd with timeout $exec_timeout"
+	echo "##########################################" >> "$export_path"
+	echo "# $description #">> "$export_path"
+	echo "# Exec [$cmd], timeout [$exec_timeout], start_time [$(date +%Y:%m:%d-%H:%M:%S)] #" >> "$export_path"
+	echo "##########################################" >> "$export_path"
+	eval timeout "${exec_timeout}" "$cmd" >> "$export_path" 2>&1
+	rc=$?
+	end_time="$(date +%s)"
+	echo "######## Execution done in [$((end_time - start_time)) ], return code $rc ######" >> "$export_path"
+
+	if [ "$rc" -eq 0 ]; then
+		log "Execution [$cmd] completed"
+	else
+		err_log "Execution [$cmd] Failed/Timeout with $rc exit code"
+	fi
+
+	echo >> "$export_path"
+}
+
+# shellcheck disable=SC2154,SC3060,SC3043
 exec_spec()
 {
-	local json_file exec_skip name timeout exec_timeout rc start_time end_time
+	local json_file exec_skip name timeout exec_timeout start_time end_time
 
 	start_time="$(date +%s)"
 	json_file="$1"
@@ -189,20 +225,18 @@ exec_spec()
 		return 1
 	}
 
-	name="$(basename ${json_file})"
+	name="$(basename "${json_file}")"
-	export_path="${REPORT_TEMP_DIR}/${name//.json/.log}"
-
 	exec_skip=0
 	if json_is_a dependency array; then
 		json_select "dependency"
 		json_get_keys ekeys
 
 		for key in $ekeys; do
-			if json_is_a $key object; then
+			if json_is_a "$key" object; then
-				json_select $key
+				json_select "$key"
 				json_get_var type type
 
-				if [ "$type" == "file" ]; then
+				if [ "$type" = "file" ]; then
 					json_get_var file file
 					if [ ! -e "$file" ]; then
 						err_log "${json_file} has unmet file dependency $file"
@@ -230,8 +264,8 @@ exec_spec()
 		json_get_keys keys
 
 		for key in $keys; do
-			if json_is_a $key object; then
+			if json_is_a "$key" object; then
-				json_select $key
+				json_select "${key}"
 				local cmd_skip file
 
 				cmd_skip=0
@@ -240,13 +274,13 @@ exec_spec()
 					json_select "dependency"
 					json_get_keys d_keys
 
-					for d_key in $d_keys; do
+					for d_key in ${d_keys}; do
-						if json_is_a $d_key object; then
+						if json_is_a "${d_key}" object; then
-							json_select $d_key
+							json_select "${d_key}"
 							json_get_var type type
-							if [ "$type" == "file" ]; then
+							if [ "$type" = "file" ]; then
 								json_get_var file file
-								if [ ! -e $file ]; then
+								if [ ! -e "${file}" ]; then
 									json_select ..
 									cmd_skip=1
 									continue
@@ -273,22 +307,7 @@ exec_spec()
 				else
 					exec_timeout=$TIMEOUT
 				fi
-				log "Executing $cmd with timeout $exec_timeout"
+				run_cmd "${exec_timeout}" "${name//.json/}" "${cmd}" "${description}"
-				echo "##########################################" >> $export_path
-				echo "# $description #">> $export_path
-				echo "# Exec [$cmd], timeout [$exec_timeout] #" >> $export_path
-				echo "##########################################" >> $export_path
-				eval timeout ${exec_timeout} $cmd >> $export_path 2>&1
-				rc=$?
-				echo "######## Execution done return code $rc ######" >> $export_path
-
-				if [ "$rc" -eq 0 ]; then
-					log "Execution [$cmd] completed"
-				else
-					err_log "Execution [$cmd] Failed/Timeout with $rc exit code"
-				fi
-
-				echo >> $export_path
 				json_select ..
 			fi
 		done
@@ -300,20 +319,21 @@ exec_spec()
 	log ""
 }
 
+# shellcheck disable=SC3043,SC3060
 generate_module()
 {
-	local modules="${@}"
+	local modules="${*}"
 	local file module
 
 	config_load
 
-	log "Modules [$@]"
+	log "Modules [$*]"
 	for module in $modules; do
 		module="${module/.json/}"
-		file="$(find $SPEC_DIR -type f -name ${module}.json)"
+		file="$(find "${SPEC_DIR}" -type f -name "${module}.json")"
 		[ -z "$file" ] && {
 			[ -d "${SPEC_EXT_DIR}" ] && \
-				file="$(find $SPEC_EXT_DIR -type f -name ${module}.json)"
+				file="$(find "${SPEC_EXT_DIR}" -type f -name "${module}.json")"
 		}
 
 		[ -f "$file" ] && \
@@ -321,15 +341,16 @@ generate_module()
 	done
 }
 
+# shellcheck disable=SC3043
 generate_all()
 {
 	local files
 
 	config_load
 
-	files="$(find ${SPEC_DIR} -type f -name *.json)"
+	files="$(find "${SPEC_DIR}" -type f -name "*.json")"
 	[ -d "${SPEC_EXT_DIR}" ] && \
-		files="${files} $(find $SPEC_EXT_DIR -type f -name *.json)"
+		files="${files} $(find "${SPEC_EXT_DIR}" -type f -name "*.json")"
 
 	[ -z "$files" ] && {
 		return 0
@@ -341,6 +362,7 @@ generate_all()
 
 }
 
+# shellcheck disable=SC3060,SC3043
 list_modules()
 {
 	local files
@@ -354,7 +376,7 @@ list_modules()
 	fi
 
 	cd ${SPEC_DIR} && {
-		for file in $(ls); do
+		for file in *.json; do
 			if [ "${JSON_OUT}" -eq 1 ]; then
 				json_add_string "" "${file/.json/}"
 			else
@@ -372,7 +394,7 @@ list_modules()
 	fi
 
 	cd ${SPEC_EXT_DIR} && {
-		for file in $(ls); do
+		for file in *.json; do
 			if [ "${JSON_OUT}" -eq 1 ]; then
 				json_add_string "" "${file/.json/}"
 			else
@@ -411,6 +433,10 @@ while getopts "m:hlj" opts; do
 		m)
 			modules="$modules ${OPTARG}"
 			;;
+		*)
+			help
+			exit
+			;;
 	esac
 done
 
@@ -422,5 +448,5 @@ fi
 if [ -z "${modules}" ]; then
 	generate_all
 else
-	generate_module ${modules}
+	generate_module "${modules}"
 fi

self-diagnostics/src/selftest.c

@@ -54,18 +54,35 @@ int get_operate_args_SelfTest(char *refparam, struct dmctx *ctx, void *data, cha
 int operate_Device_SelfTest(char *refparam, struct dmctx *ctx, void *data, char *instance, char *value, int action)
 {
 	char cmd[512] = {0};
-	char output[512] = {0};
+	char buffer[512] = {0};
+	json_object *jobj = NULL;
+	const char *filename;
 
-	snprintf(cmd, sizeof(cmd), "sh %s", DIAG_BIN);
+	snprintf(cmd, sizeof(cmd), "sh %s -j 2>/dev/null", DIAG_BIN);
 
-	if (run_cmd(cmd, output, sizeof(output)) != 0)
+	if (run_cmd(cmd, buffer, sizeof(buffer)) != 0) {
+		BBFDM_ERR("Failed to run cmd[%s]", cmd);
 		goto err;
+	}
 
-	// truncate the new line char from end
+	if (DM_STRLEN(buffer) == 0) {
-	remove_new_line(output);
+		BBFDM_ERR("No output from cmd[%s]", cmd);
-
-	if (!file_exists(output))
 		goto err;
+	}
+
+	jobj = json_tokener_parse(buffer);
+	if (jobj == NULL) {
+		BBFDM_ERR("Fail to parse output[%s] in json", buffer);
+		goto err;
+	}
+
+	filename = dmjson_get_value(jobj, 1, "result");
+	snprintf(buffer, sizeof(buffer), "%s", filename);
+	json_object_put(jobj);
+	if (!file_exists(filename)) {
+		BBFDM_ERR("File [%s] does not exists or not accessible", filename);
+		goto err;
+	}
 
 	/* Add in dmmap_logmngr */
 	struct uci_section *s = get_origin_section_from_dmmap("dmmap_logmngr", "self_test", "self_test_log");
@@ -74,7 +91,7 @@ int operate_Device_SelfTest(char *refparam, struct dmctx *ctx, void *data, char
 		dmuci_rename_section_by_section(s, "self_test_log");
 	}
 
-	dmuci_set_value_by_section(s, "log_file", output);
+	dmuci_set_value_by_section(s, "log_file", filename);
 	dmuci_commit_package_bbfdm("dmmap_logmngr");
 
 	/* Get self test log instance */