Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link
Community Bot
Community Bot
  • 1

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 <stdlib.h>  // for getenv(), srand(), fprintf(), etc.
#include <stdbool.h>  // 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 answerthis answer for more details on the general topic of "things that might happen to an open file."

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 <stdlib.h>  // for getenv(), srand(), fprintf(), etc.
#include <stdbool.h>  // 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."

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 <stdlib.h>  // for getenv(), srand(), fprintf(), etc.
#include <stdbool.h>  // 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."

added info about deleting open files
Source Link
Edward
Edward
  • 66.6k
  • 4
  • 118
  • 282

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 <stdlib.h>  // for getenv(), srand(), fprintf(), etc.
#include <stdbool.h>  // 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."

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 <stdlib.h>  // for getenv(), srand(), fprintf(), etc.
#include <stdbool.h>  // 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.

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 <stdlib.h>  // for getenv(), srand(), fprintf(), etc.
#include <stdbool.h>  // 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."

Source Link
Edward
Edward
  • 66.6k
  • 4
  • 118
  • 282

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 <stdlib.h>  // for getenv(), srand(), fprintf(), etc.
#include <stdbool.h>  // 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.