Fetch a Quranic Verse from the Web

Question

Verse is a command-line program that allows you to retrieve specific verses from the Quran. It takes a chapter and verse number as input and provides you with the corresponding Quranic verse.

Dependencies:

• libcurl
• libjansson

strtoi.h:

#ifndef STRTOI_H
#define STRTOI_H
typedef enum {
 STRTOI_SUCCESS,
 STRTOI_OVERFLOW,
 STRTOI_UNDERFLOW,
 STRTOI_INCONVERTIBLE
} strtoi_errno;
/**
* @brief strtoi() shall convert string nptr to int out. 
*
* @param nptr - Input string to be converted.
* @param out - The converted int.
* @param base - Base to interpret string in. Same range as strtol (2 to 36).
*
* The format is the same as strtol, except that the following are inconvertible:
* 
* - empty string
* - leading whitespace
* - any trailing characters that are not part of the number
*
* @return Indicates if the operation succeeded, or why it failed. 
*/
strtoi_errno strtoi(int *restrict out, const char *restrict nptr, int base);
#endif /* STRTOI_H */

strtoi.c:

#include "strtoi.h"
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <limits.h>
#include <ctype.h>
strtoi_errno strtoi(int *restrict out, const char *restrict nptr, int base)
{
 /*
 * Null string, empty string, leading whitespace? 
 */
 if (!nptr || !nptr[0] || isspace(nptr[0])) {
 return STRTOI_INCONVERTIBLE;
 }
 char *end_ptr;
 const int errno_original = errno; /* We shall restore errno to its original value before returning. */
 const long int i = strtol(nptr, &end_ptr, base);
 errno = 0;
 /*
 * Both checks are needed because INT_MAX == LONG_MAX is possible.
 */
 if (i > INT_MAX || (errno == ERANGE && i == LONG_MAX)) {
 return STRTOI_OVERFLOW;
 } else if (i < INT_MIN || (errno == ERANGE && i == LONG_MIN)) {
 return STRTOI_UNDERFLOW;
 } else if (*end_ptr || nptr == end_ptr) {
 return STRTOI_INCONVERTIBLE;
 }
 *out = (int) i;
 errno = errno_original;
 return STRTOI_SUCCESS;
}

errors.h:

#ifndef ERRORS_H
#define ERRORS_H
#include <stddef.h>
#define ARRAY_CARDINALITY(x) (sizeof (x) / sizeof ((x)[0]))
/*
 * Error codes for invalid arguments.
 */
enum error_codes {
 E_SUCCESS = 0,
 E_NULL_ARGV,
 E_INSUFFICIENT_ARGS,
 E_INVALID_CHAPTER,
 E_INVALID_VERSE,
 E_INVALID_RANGE,
 E_PARSE_ERROR,
 E_ENOMEM,
 E_UNKNOWN,
 E_CURL_INIT_FAILED,
 E_CURL_PERFORM_FAILED
};
/**
* @brief get_err_msg() shall retrieve the error message corresponding to the given error code. 
* 
* @param err_code - An integer respresenting the error code.
* 
* @return A pointer to a constant string containing the error message.
* If the error code is not recognized, a default "Unknown error code.\n" 
* message is returned.
*/
const char *get_err_msg(int err_code);
#endif /* ERRORS_H */

errors.c:

#include "errors.h"
#include <assert.h>
/* 
 * Array of strings to map enum error types to printable string. 
 */
static const char *const errors[] = {
 /* *INDENT-OFF* */
 [E_NULL_ARGV] =
 "Error: A NULL argv[0] was passed through an exec system call.\n",
 [E_INSUFFICIENT_ARGS] =
 "Usage: verse <chapter> <verse>\n",
 [E_INVALID_CHAPTER] = 
 "Error: Invalid chapter number.\n",
 [E_INVALID_VERSE] = 
 "Error: Invalid verse number for the given chapter.\n",
 [E_INVALID_RANGE] = 
 "Error: Chapter or verse out of valid numeric range.\n",
 [E_PARSE_ERROR] = 
 "Error: Non-numeric input for chapter or verse.\n",
 [E_ENOMEM] = 
 "Error: Insufficient memory.\n",
 [E_UNKNOWN] = 
 "Fatal: An unknown error has arisen.\n",
 [E_CURL_INIT_FAILED] = 
 "Error: curl_easy_init() failed.\n",
 [E_CURL_PERFORM_FAILED] = 
 "Error: curl_easy_perform() failed.\n"
 /* *INDENT-ON* */
};
const char *get_err_msg(int err_code)
{
 static_assert(ARRAY_CARDINALITY(errors) - 1 == E_CURL_PERFORM_FAILED,
 "The errors array and the enum must be kept in-sync!");
 if (err_code >= 0 && err_code < (int) ARRAY_CARDINALITY(errors)) {
 return errors[err_code];
 }
 return "Unknown error code.\n";
}

webutil.h:

#ifndef WEB_UTIL_H
#define WEB_UTIL_H
#include <stddef.h>
#include <curl/curl.h>
/*
 * A struct to hold the contents of the downloaded web page. 
 */
struct mem_chunk {
 char *ptr;
 size_t len;
};
/**
* @brief Parses a JSON response to extract a specific verse.
*
* @param json_responose - A JSON response string containing verse data.
* @param out - A pointer to a string where the parsed verse will be stored.
* 
* @return An error code indicating the success or failure of the parsing process.
* If successful, the parsed verse will be stored in the 'out' parameter.
*/
int parse_response_json(const char *restrict json_response,
 char **restrict out);
/**
* @brief A callback function for curl_easy_perform(). Stores the downloaded
* web content to the mem_chunk struct as it arrives.
* @param Refer to https://curl.se/libcurl/c/CURLOPT_WRITEFUNCTION.html for a 
* detailed explanation about the parameters.
* @return The number of bytes read, or 0 on a memory allocation failure.
*/
size_t write_memory_callback(void *content, size_t size,
 size_t nmemb, struct mem_chunk *chunk);
/**
* @brief download_webpage() shall download the contents of a web page specified
* by the URL using libcurl.
* 
* @param chunk - A struct to hold the downloaded content.
* @param curl - A libcurl handle for performing the download.
* @param url - The URL of the web page to download.
* 
* @return CURLE_OK on success, or a libcurl error code on failure.
*/
int download_webpage(const struct mem_chunk *restrict chunk,
 CURL * restrict curl, const char *restrict url);
#endif /* WEB_UTIL_H */

web_util.c:

#ifndef _XOPEN_SOURCE
#define _XOPEN_SOURCE 700
#endif
#include "web_util.h"
#include "errors.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include <jansson.h>
#define MAX_VERSE_SIZE 4096
int parse_response_json(const char *restrict json_response, char **restrict out)
{
 json_error_t error;
 json_t *const root = json_loads(json_response, 0, &error);
 if (root) {
 /*
 * Check if "text" exists in the JSON structure. 
 */
 const json_t *const data = json_object_get(root, "data");
 const json_t *const text = json_object_get(data, "text");
 if (data && text && json_is_string(text)) {
 *out = strdup(json_string_value(text));
 } else {
 json_decref(root);
 return E_UNKNOWN;
 }
 } else {
 fputs(error.text, stderr);
 return E_UNKNOWN;
 }
 json_decref(root);
 return E_SUCCESS;
}
size_t
write_memory_callback(void *content, size_t size,
 size_t nmemb, struct mem_chunk *chunk)
{
 const size_t new_size = chunk->len + size * nmemb;
 void *const cp = realloc(chunk->ptr, new_size + 1);
 if (!cp) {
 perror("realloc()");
 return 0;
 }
 chunk->ptr = cp;
 memcpy(chunk->ptr + chunk->len, content, size * nmemb);
 chunk->ptr[new_size] = '0円';
 chunk->len = new_size;
 return size * nmemb;
}
int
download_webpage(const struct mem_chunk *restrict chunk,
 CURL * restrict curl, const char *restrict url)
{
 CURLcode ret;
 /* *INDENT-OFF* */
 if ((ret = curl_easy_setopt(curl, CURLOPT_URL, url)) != CURLE_OK
 || (ret = curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_memory_callback)) != CURLE_OK
 || (ret = curl_easy_setopt(curl, CURLOPT_WRITEDATA, chunk)) != CURLE_OK
 || (ret = curl_easy_setopt(curl, CURLOPT_USERAGENT, "Verse/1.0")) != CURLE_OK
 || (ret = curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L)) != CURLE_OK
 || (ret = curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 5L)) != CURLE_OK) {
 return (int) ret;
 }
 /* *INDENT-ON* */
 return (int) curl_easy_perform(curl);
}

main.c:

#ifdef _POSIX_C_SOURCE
#undef _POSIX_C_SOURCE
#endif
#ifdef _XOPEN_SOURCE
#undef _XOPEN_SOURCE
#endif
#define _POSIX_C_SOURCE 200819L
#define _XOPEN_SOURCE 700
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "strtoi.h"
#include "web_util.h"
#include "errors.h"
#define BASE_URL "http://api.alquran.cloud/v1/ayah/%d:%d/en.maududi"
#define MAX_URL_SIZE 128
#define MAX_CHAPTER 114
#define MIN_CHAPTER 0
#define MIN_VERSE 0
#define INIT_MEM_CHUNK(address, size) \
 { .ptr = address, .len = size }
/*
 * Each entry in the table is the maximum number of verses present in its 
 * corresponding index, which denotes the chapter number. 
 */
static const int verse_limits[] = {
 7, 286, 200, 176, 120, 165, 206, 75, 129, 109, 123, 111, 43, 52,
 99, 128, 111, 110, 98, 135, 112, 78, 118, 64, 77, 227, 93, 88, 69,
 60, 34, 30, 73, 54, 45, 83, 182, 88, 75, 85, 54, 53, 89, 59, 37,
 35, 38, 29, 18, 45, 60, 49, 62, 55, 78, 96, 29, 22, 24, 13, 14,
 11, 11, 18, 12, 12, 30, 52, 52, 44, 28, 28, 20, 56, 40, 31, 50,
 40, 46, 42, 29, 19, 36, 25, 22, 17, 19, 26, 30, 20, 15, 21, 11,
 8, 8, 19, 5, 8, 8, 11, 11, 8, 3, 9, 5, 4, 7, 3, 6, 3, 5, 4, 5, 6
};
static inline int check_args(int argc, const char *const *argv)
{
 /*
 * Sanity check. POSIX requires the invoking process to pass a non-NULL argv[0]. 
 */
 return (!argv[0]) ? E_NULL_ARGV :
 (argc != 3) ? E_INSUFFICIENT_ARGS : E_SUCCESS;
}
static int check_input(const char *const *restrict argv, int *restrict chapter,
 int *restrict verse)
{
 const int ret_1 = strtoi(chapter, argv[1], 10);
 const int ret_2 = strtoi(verse, argv[2], 10);
 /* *INDENT-OFF* */
 return (ret_1 == STRTOI_INCONVERTIBLE || ret_2 == STRTOI_INCONVERTIBLE) ? E_PARSE_ERROR :
 (ret_1 == STRTOI_UNDERFLOW || ret_2 == STRTOI_UNDERFLOW ||
 ret_1 == STRTOI_OVERFLOW || ret_2 == STRTOI_OVERFLOW) ? E_INVALID_RANGE :
 (*chapter <= MIN_CHAPTER || *chapter > MAX_CHAPTER) ? E_INVALID_CHAPTER :
 (*verse <= MIN_VERSE || *verse > verse_limits [*chapter - 1]) ? E_INVALID_VERSE :
 E_SUCCESS;
 /* *INDENT-ON* */
}
static int handle_args(int chapter, int verse)
{
 char url[MAX_URL_SIZE];
 snprintf(url, sizeof (url), BASE_URL, chapter, verse);
 struct mem_chunk chunk = INIT_MEM_CHUNK(0, 0);
 CURL *const curl = curl_easy_init();
 if (!curl) {
 return E_CURL_INIT_FAILED;
 }
 int rc = download_webpage(&chunk, curl, url);
 if (rc != CURLE_OK) {
 curl_easy_cleanup(curl);
 free(chunk.ptr);
 return E_CURL_PERFORM_FAILED;
 } else {
 char *result = NULL;
 rc = parse_response_json(chunk.ptr, &result);
 if (rc != E_SUCCESS) {
 curl_easy_cleanup(curl);
 free(chunk.ptr);
 return rc;
 } else {
 printf("(%d:%d) %s\n", chapter, verse, result);
 }
 free(result);
 }
 curl_easy_cleanup(curl);
 curl_global_cleanup();
 free(chunk.ptr);
 return E_SUCCESS;
}
int main(int argc, char **argv)
{
 const char *const *args = (const char *const *) argv;
 int status = check_args(argc, args);
 if (status != E_SUCCESS) {
 fputs(get_err_msg(status), stderr);
 return EXIT_FAILURE;
 }
 int chapter, verse;
 status = check_input(args, &chapter, &verse);
 if (status != E_SUCCESS) {
 fputs(get_err_msg(status), stderr);
 return EXIT_FAILURE;
 }
 status = handle_args(chapter, verse);
 if (status != E_SUCCESS) {
 fputs(get_err_msg(status), stderr);
 return EXIT_FAILURE;
 }
 return EXIT_SUCCESS;
}

Or you could clone this repository: verse

Review Goals:

• General coding comments, style, etc.

• Does any part of my code exhibit undefined/implementation-defined behavior?

• Is there a better way to structure the code?

• qwr: This is a bit off topic but do you have to use C? Web requests and API interaction can be done much easier in python (it would fit in 1 file what takes 7 here), and speed doesn't seem to be an issue here.
• Madagascar: @qwr No, I don't have to use C. The code isn't recent, and the primary focus was to better familiarise myself with libcurl. I also didn't use Python some months ago.
• qwr: My non-answer would be to rewrite in python, which would automatically make the code have more predictable error handling and easier to maintain.
• pacmaninbw: @qwr Coding for performance might require C over Python.
• qwr: @pacmaninbw I agree, but in that case I would skip the API entirely since the Quran can fit in a modestly sized local file. Actually, I would do that with python too.

Answer 1

Just a strtoi() review.

Bug: errno setting/restoration woes.

errno = 0; should be done before calling strtol();

Below code may set errno due to strtol(), yet the next line is errno = 0; and following tests like errno == ERANGE are always false.

// Problem code
const int errno_original = errno;
const long int i = strtol(nptr, &end_ptr, base);
errno = 0; // ????
if (i > INT_MAX || (errno == ERANGE && i == LONG_MAX)) {

Instead, sample errno and then restore it. Use sample for later tests.

// Sample fix
const int errno_original = errno;
errno = 0;
const long int i = strtol(nptr, &end_ptr, base);
int errno_sample = errno;
errno = errno_original;
if (i > INT_MAX || (errno_sample == ERANGE && i == LONG_MAX)) {

Code does not restore errno in select cases

Consider moving errno = errno_original; up near strtol() to restore errno along all code paths.

Unneeded test

// v------v not needed. `strtol()` handles that. 
// if (!nptr || !nptr[0] || isspace(nptr[0])) {
if (!nptr || isspace(nptr[0])) {

and re-organize tests

} if (*end_ptr || nptr == end_ptr) {
 return STRTOI_INCONVERTIBLE;
} else if (i > INT_MAX || (errno == ERANGE && i == LONG_MAX)) {
 return STRTOI_OVERFLOW;
} else if (i < INT_MIN || (errno == ERANGE && i == LONG_MIN)) {
 return STRTOI_UNDERFLOW;
}

White space test potentially out of range

is...() functions need an unsigned char value or EOF. When char is signed, code risks UB.

// if (!nptr || !nptr[0] || isspace(nptr[0])) {
if (!nptr || !nptr[0] || isspace((unsigned char)nptr[0])) {

strtoi() differs from strtol() on extremes

Consider making strtoi() more strtol() like by setting *nptr in all cases like INT_MIN, INT_MAX, 0 on too low, too high or no convert.

strtoi() name

strtoi() is a reserved name:

Function names that begin with str and a lowercase letter may be added to the declarations in the <stdlib.h> header. C11 7.31.12 1

Consider a new name, maybe str2i()?

Consider STRTOI_N

Creating STRTOI_N can simplify testing the function result for range.

typedef enum {
 STRTOI_SUCCESS,
 STRTOI_OVERFLOW,
 STRTOI_UNDERFLOW,
 STRTOI_INCONVERTIBLE,
 STRTOI_N // add
} strtoi_errno;

Mis-comments

 @param base - Base to interpret string in. Same range as strtol (2 to 36).

should be

 @param base - Base to interpret string in. Same range as strtol (0 and 2 to 36).

- empty string not needed in .h exception list.

base out of range?

Interesting that strtoi() with its many checks does not test base. C does not specify a base check for strtol().

Unneeded else

A style issue.

if (i > INT_MAX || (errno == ERANGE && i == LONG_MAX)) {
 return STRTOI_OVERFLOW;
// else not needed here.
} else if (i < INT_MIN || (errno == ERANGE && i == LONG_MIN)) {
 return STRTOI_UNDERFLOW;
}

Minor: unnecessary #include <*.h>

strtoi.c does not need #include <stdio.h>.

Even thought this does not apply to strtoi.c, for user.h files, I strongly recommend to only include necessary #include <*.h> files.

For user.c files, the issue is less important.
I consider #include <*.h> files in a user.c file as not a real issue as there exist a reasonable argument to include some <*.h> to make certain the user.c code does not conflict with the unnecessary #include <*.h>. Further, the maintenance needed to only include a limit set in a user.c file is not that productive. Note: that some IDEs offer an editing option to include the minimum set. Something of a spell checker for #include.

This issue is a bit of a software war and best to follow your group's coding standards.

---

Candidate replacement (untested):

str2i_error str2i(int *restrict out, const char *restrict nptr, int base) {
 
 // Maybe test out
 if (out == NULL) {
 return STR2I_INCONVERTIBLE;
 }
 if (nptr == NULL || isspace((unsigned char) nptr[0])) {
 *out = 0;
 return STR2I_INCONVERTIBLE;
 }
 if (!(base == 0 || (base >= 2 && base <= 36))) {
 *out = 0;
 return STR2I_INCONVERTIBLE;
 }
 char *end_ptr;
 int errno_original = errno;
 errno = 0;
 // I used value here rather than i since it is not an int.
 long value = strtol(nptr, &end_ptr, base);
 int errno_sample = errno;
 errno = errno_original;
 if (*end_ptr || nptr == end_ptr) {
 *out = 0;
 return STR2I_INCONVERTIBLE;
 }
 if (value > INT_MAX || (errno_sample == ERANGE && value == LONG_MAX)) {
 *out = INT_MAX;
 return STR2I_OVERFLOW;
 }
 if (value < INT_MIN || (errno_sample == ERANGE && value == LONG_MIN)) {
 *out = INT_MIN;
 return STR2I_UNDERFLOW;
 }
 *out = (int) value;
 return STR2I_SUCCESS;
}

---

Consider making a str2i() that works just like strtol(), expect for range. Example Then form your str2i_error() which uses str2i(). Easier to extend and make your str2other_error() functions.

Advanced

Even consider _Generic.

Answer 2

General Observations

There are definitely some good points to this code, for example rather than write your own code to do the curl functions or the JSON manipulation you used libraries. That is definitely the way to go.

The code in your Simple Shell in C question is better.

The error strings in error.c could be more maintainable.

Modularization

Generally one wants to limit the scope of include files such as <curl/curl.h>. All dependencies on curl.h should be in web_util.c and curl.h should be included by web_util.c not web_util.h. This means that there should be a function prototype for int handle_args(int chapter, int verse) web_util.h. The function prototypes that are currently in web_util.h should all bemoved to web_util.c and should probably be declared as static.

Among other things, this might make the code in web_util.c reusable, and more portable. It would also mean that the mem_chunk struct no longer needs to be shared with the rest of the program.

Extensible Code

The error checking in check_input() is very difficult to modify and maintain. It also isn't exactly easy to read.

What I might change is to add a second public interface to strtoi.h and strtoi.c. This second public function would return the error codes from error.h for strtoi(). This would limit the code in int check_input() to just checking the validity of the verse and the chapter.

The fact that you had to turn off the indentation in this function is a indication that there was a problem in the function.

Improved Explanation

The comment for verse_limits[] isn't clear enough, explain more about where the data came from and what each number means.

struct Initialization

In the function int handle_args(int chapter, int verse) the macro

#define INIT_MEM_CHUNK(address, size) \
 { .ptr = address, .len = size }

is used.

Rather than initializing each field in a struct to zero the code would be easier to maintain and modify if void *memset( void *dest, int ch, size_t count ) was used.

 memset(&chunk, 0, sizeof(chunk));

This will set all of the fields in chunk to zero, even if more fields are added.

The memset() and memcpy() functions are defined in string.h

Again this makes the code more extensible.