HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

Where it all begins: Khronos

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
wherekhronosbeginsall

Problem

This is the main.c file for Khronos, a personal project of mine. This is basically where the whole program is setup, run and then quit. I'll post the short summery of what this file does from the README.md:

How Khronos Works


The process starts off by recording a .flac file with the help of
LibSndFile and PortAudio working together. PortAudio scans for the
default input device (set by your operating system), and starts the
audio recording process. Once finished, PortAudio passes along the
raw PCM data that was just recorded to LibSndFile, which performs the
necessary instructions to store it in a .flac file.


This .flac file is then sent off to Google for speech processing.
We specify what the sample rate is of the audio, and what sort of
response we would like back from Google (JSON, XML, etc.). Google
sends back a nicely packed JSON file for processing to extract what
was said during the recording. This processing is done with my custom JSON parser that has proven to be faster when compared to the
benchmarks of other JSON parsers.


Khronos then takes this processed information and responds to it in an
appropriate way with it's own speech synthesis engine.

So here's what I would like reviewed:

-
Readability: is everything easily understandable to you? Can you flow through the code with ease?

-
Bottlenecks: are there places where I could improve speed?

-
Memory management: in general I try to avoid having to manage memory manually, as it can be easy to screw up and it's better for the machine to handle most of it in my opinion. Do I go too far with that concept here?

-
Portability: this code is being designed with the concept of being portable and runnable on a wide range of devices (currently it is being developed and tested using an older version Raspberry Pi). Am I constraining that in any way here?

Understandably there are parts to this code that I haven't included (some method calls, struct

Solution

Without more of the code, it's not really going to be possible to address memory management or bottlenecks. So instead, I'll look instead at readability and portability.

Use the appropriate #includes

This program fragment requires two headers, which should be included but are not:

#include   // for getenv(), srand(), fprintf(), etc.
#include   // for "true" and "false" and "bool"


Use only standard calls for portable code

The mkstemps routine is common to many Unix-like environments but is not standard -- not even POSIX. The mktemp routine is part of the C standard but has security problems and should not be used. mkstemp is a POSIX standard, so it may be your best bet.

Add more error checking

The calls to snprintf and strtod can fail. In both cases, the code should check for errors and handle them as may be appropriate to your situation.

Don't abuse the ternary operator

The code currently has this line

if (text) confidence = strtod(parcel_getItemFromJSON(resp->data, "confidence") ?: "0", NULL) * 100;


First, that would be a lot more readable if it didn't pack everything on a single line. Second, omitting the second operand is a GNU extension and not portable.

Break things up into functions

The main routine has quite a lot going on inside of it that would benefit from being broken out into separate routines. Your English language description is quite clear and suggests a way that this could be broken into separate functions.

Use unlink instead of remove

Both are defined in the standard, but remove will remove a directory just as happily as it will remove a file. If you wish only to remove a file, it's safer to use unlink.

Don't erase an open file

The use of remove as mentioned above, precedes closing the file. That is not a good idea. Better is to close the file first and then delete it. I've been asked about this, so I'm providing more detail. It's counterintuitive, but many Linux filesystems allow deleting the file before closing the file. However, we can't count on this behavior (e.g. the file could be mounted via NFS) and the possible problems are easily avoided by rearranging the code. See this answer for more details on the general topic of "things that might happen to an open file."

Code Snippets

#include <stdlib.h>  // for getenv(), srand(), fprintf(), etc.
#include <stdbool.h>  // for "true" and "false" and "bool"
if (text) confidence = strtod(parcel_getItemFromJSON(resp->data, "confidence") ?: "0", NULL) * 100;

Context

StackExchange Code Review Q#114509, answer score: 12

Revisions (0)

No revisions yet.