26
\$\begingroup\$

I programmed a generic queue in pure C and I'd love to receive some feedback from other programmers.

You may use the following questions to guide your review.

  • Is the API well thought and idiomatic?
  • Does the code look "modern" (i.e. uses modern conventions etc)?
  • Is the code efficient in terms of memory usage?
  • Are the tests well thought?
  • Is the code free of undefined behavior, memory leaks etc?

My program is divided in 3 files:

queue.h

#pragma once

#include <stddef.h>

typedef enum queue_ret
{
    QUEUE_RET_Success,
    QUEUE_RET_FailedMemoryAlloc,
    QUEUE_RET_QueueIsEmpty,
} queue_ret_t;

typedef struct node
{
    void* data;
    struct node* next;
} node_t;

typedef struct queue
{
    node_t* front;
    node_t* back;
    size_t size;
} queue_t;

queue_ret_t QUEUE_initialize(queue_t** queue);

size_t QUEUE_size(queue_t* queue);

queue_ret_t QUEUE_enqueue(queue_t* queue, void* data);

queue_ret_t QUEUE_peek(queue_t* queue, void* data, size_t size);

queue_ret_t QUEUE_dequeue(queue_t* queue, void* data, size_t size);

void QUEUE_free(queue_t* queue);

queue.c

#include <stdlib.h>
#include <string.h>

#include "queue.h"

queue_ret_t QUEUE_initialize(queue_t** queue)
{
    *queue = (queue_t*)calloc(1, sizeof(queue_t));

    if ((*queue) == NULL)
    {
        return QUEUE_RET_FailedMemoryAlloc;
    }

    (*queue)->front = NULL;
    (*queue)->back = NULL;
    (*queue)->size = 0;

    return QUEUE_RET_Success;
}

size_t QUEUE_size(queue_t* queue)
{
    return queue->size;
}

queue_ret_t QUEUE_enqueue(queue_t* queue, void* data)
{
    node_t* node = (node_t*)calloc(1, sizeof(node_t));

    if (node == NULL)
    {
        return QUEUE_RET_FailedMemoryAlloc;
    }

    node->data = data;
    node->next = NULL;

    if (queue->size == 0)
    {
        queue->front = node;
        queue->back = node;
    }
    else
    {
        queue->back->next = node;
        queue->back = node;
    }

    (queue->size)++;

    return QUEUE_RET_Success;
}

queue_ret_t QUEUE_peek(queue_t* queue, void* data, size_t size)
{
    if (queue->size == 0)
    {
        return QUEUE_RET_QueueIsEmpty;
    }

    memcpy(data, queue->front->data, size);

    return QUEUE_RET_Success;
}

queue_ret_t QUEUE_dequeue(queue_t* queue, void* data, size_t size)
{
    queue_ret_t ret = QUEUE_peek(queue, data, size);

    if (ret != QUEUE_RET_Success)
    {
        return ret;
    }

    if (queue->front == queue->back)
    {
        free(queue->front);
        queue->front = NULL;
        queue->back = NULL;
    }
    else
    {
        node_t* temp = queue->front;
        queue->front = temp->next;
        free(temp);
    }

    (queue->size)--;

    return QUEUE_RET_Success;
}

void QUEUE_free(queue_t* queue)
{
    node_t* current = queue->front;

    while (current != NULL)
    {
        node_t* next = current->next;
        free(current);
        current = next;
    }

    free(queue);
}

queue_test.c

#include "greatest.h"
#include "queue.h"

TEST dequeue_empty_queue_should_return_QueueIsEmpty()
{
    queue_t* queue;
    queue_ret_t ret;

    ret = QUEUE_initialize(&queue);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_dequeue(queue, NULL, 0);

    ASSERT_EQ(QUEUE_RET_QueueIsEmpty, ret);

    QUEUE_free(queue);

    PASS();
}

TEST peek_empty_queue_should_return_QueueIsEmpty()
{
    queue_t* queue;
    queue_ret_t ret;

    ret = QUEUE_initialize(&queue);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_peek(queue, NULL, 0);

    ASSERT_EQ(QUEUE_RET_QueueIsEmpty, ret);

    QUEUE_free(queue);

    PASS();
}

TEST size_of_empty_queue_should_be_0()
{
    queue_t* queue;
    queue_ret_t ret;
    size_t actual;

    ret = QUEUE_initialize(&queue);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    actual = QUEUE_size(queue);

    ASSERT_EQ(0, actual);

    QUEUE_free(queue);

    PASS();
}

TEST enqueue_should_make_size_grow()
{
    queue_t* queue;
    queue_ret_t ret;
    int dummy = 0;
    size_t actual;

    ret = QUEUE_initialize(&queue);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_enqueue(queue, &dummy);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_enqueue(queue, &dummy);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_enqueue(queue, &dummy);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    actual = QUEUE_size(queue);

    ASSERT_EQ(3, actual);

    QUEUE_free(queue);

    PASS();
}

TEST peek_should_return_next_dequeue_item()
{
    queue_t* queue;
    queue_ret_t ret;
    int expected = 1;
    int dummy = 2;
    int actual = 0;

    ret = QUEUE_initialize(&queue);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_enqueue(queue, &expected);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_peek(queue, &actual, sizeof(actual));

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ASSERT_EQ(expected, actual);

    ret = QUEUE_enqueue(queue, &dummy);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_peek(queue, &actual, sizeof(actual));

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ASSERT_EQ(expected, actual);

    QUEUE_free(queue);

    PASS();
}

TEST dequeue_all_items_should_left_queue_empty()
{
    queue_t* queue;
    queue_ret_t ret;
    int dummy = 0;
    size_t actual;

    ret = QUEUE_initialize(&queue);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_enqueue(queue, &dummy);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_enqueue(queue, &dummy);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_dequeue(queue, &dummy, sizeof(dummy));

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_dequeue(queue, &dummy, sizeof(dummy));

    ASSERT_EQ(QUEUE_RET_Success, ret);

    actual = QUEUE_size(queue);

    ASSERT_EQ(0, actual);

    QUEUE_free(queue);

    PASS();
}

TEST first_item_in_should_be_first_item_out()
{
    queue_t* queue;
    int expected_1 = 1;
    int expected_2 = 2;
    int actual;
    queue_ret_t ret;

    ret = QUEUE_initialize(&queue);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_enqueue(queue, &expected_1);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_enqueue(queue, &expected_2);

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ret = QUEUE_dequeue(queue, &actual, sizeof(actual));

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ASSERT_EQ(expected_1, actual);

    ret = QUEUE_dequeue(queue, &actual, sizeof(actual));

    ASSERT_EQ(QUEUE_RET_Success, ret);

    ASSERT_EQ(expected_2, actual);

    QUEUE_free(queue);

    PASS();
}

GREATEST_MAIN_DEFS();

int main(int argc, char **argv) {
    GREATEST_MAIN_BEGIN();

    RUN_TEST(dequeue_empty_queue_should_return_QueueIsEmpty);
    RUN_TEST(peek_empty_queue_should_return_QueueIsEmpty);
    RUN_TEST(size_of_empty_queue_should_be_0);
    RUN_TEST(enqueue_should_make_size_grow);
    RUN_TEST(peek_should_return_next_dequeue_item);
    RUN_TEST(dequeue_all_items_should_left_queue_empty);
    RUN_TEST(first_item_in_should_be_first_item_out);

    GREATEST_MAIN_END();
}

How to run

Put all the files shown above in the same folder. Then, you need to copy the testing library to the this folder, which is just a single header file called greatest.h.

So, assuming your folder looks like this:

> ls
> greatest.h  queue.c  queue.h  queue_test.c

You run the tests with:

gcc queue.c queue_test.c -o queue_test && queue_test
\$\endgroup\$
  • \$\begingroup\$ the posted code is missing the header file contents for greatest.h Please edit the question to include that missing header file \$\endgroup\$ – user3629249 May 15 at 15:53
  • \$\begingroup\$ @user3629249 I included a link to it, which is: raw.githubusercontent.com/silentbicycle/greatest/master/… \$\endgroup\$ – Gabriel May 15 at 16:01
  • \$\begingroup\$ @user3629249 It looks like you didn't copy the content of greatest.h. All I have to do is to create a greatest.h file with the content found in the previous link, so I presume this is all you have to do too. \$\endgroup\$ – Gabriel May 15 at 16:07
  • \$\begingroup\$ After copying the linked file: greatest.h it cleanly compiles. DO NOT reference links to external files. The posted code should stand on its' own \$\endgroup\$ – user3629249 May 15 at 16:12
  • 1
    \$\begingroup\$ This looks like a linked list trying to get out! I would extract out the linked list code (which could then be used directly, or as part of a stack or other data structure), and then wrap it to create the queue and expose only those operations which are legal for queues. \$\endgroup\$ – Alexander May 18 at 15:28
22
\$\begingroup\$

None of your QUEUE_* functions validate their input arguments before using them. NULL pointers will be a problem, particularly for the pointer-to-pointer arguments.

C's memory-allocation functions return a void*, which implicitly converts to any other pointer type. This means that you don't need to typecast the result of calloc. Doing so can actually mask certain errors that the compiler would otherwise catch.

Your node_t structure is missing a vital piece of information: the size of the target object. This would be okay if your queue dealt purely with pointers. In several places, though, you access the actual data behind that pointer. Without knowing the size of that data, you're opening up yourself to tons of buffer overrun problems. For example, if I queue a char and then call QUEUE_peek() with a size of 1 MB, bad things happen. Trusting the caller to keep track of this is problematic. First and foremost, you shouldn't trust the user to do the right thing. More importantly, requiring a second queue to keep track of object sizes in the first queue makes your implementation difficult to use. You have two options for resolving this. If the queue will only hold objects of the same type, replace void* data with a pointer to a specific type (clever macros can even make this type user-configurable like a C++ template). The other option is to add a size_t size member to struct node. If you go with the latter, you'll need to either re-design peek/dequeue to allocate the output buffer internally, or add a function that returns the size of the object at the head of the queue.

Your code is very vulnerable to what I sometimes call the "nosy user". That is, a user that pokes around at the internals of your implementation. If a user decides to alter the value of a queue's size member (either through malice or incompetence), your code will break in a number of places. I recommend hiding the implementation details from the user. The easiest way to do this would be to move the definitions of struct queue and struct node into the .c file. Then, add the line struct queue; to the top of the header (a forward declaration). This allows the user to create pointers to queue objects - which is all they need in order to use your API - but doesn't let them poke about at the internals. This is similar to how stdio.h implements the FILE type. Side note: nice work on the clean API that only requires the user to know about a single pointer type. Most first passes leak a lot more implementation details than that.

Expanding on what ratchet freak mentioned above, your API is asymmetrical. You enqueue a pointer to an object, but you dequeue a copy of the content of the object. This can cause problems when you queue objects that have been dynamically allocated. Once you dequeue that object and want to free it, you have no real way of figuring out the original pointer so that you can pass it to free(). If the dequeue function returned the pointer instead, the caller would have all of the information they'd need.

Having enqueue/dequeue only deal in pointers would avoid another problem as well. The current dequeue implementation makes a complete copy of the queued object, which can have severe performance penalties. Using large objects with your queue would lead to a lot of memory churn plus a lot of time wasted copying bits unnecessarily. Users could always queue pointers to these objects instead of the objects themselves, but that's more of a workaround than a solution.

Portability issues:

The POSIX standard reserves typenames that end in _t. For the sake of portability, it's best if you avoid using that suffix on your custom types. The risk of name collisions is particularly high when you have basic, generic names like queue_t and node_t.

#pragma once is not standard C. It's a relatively common extension but if you want to ensure that your code works with any compiler/platform, use a traditional #define-based include guard instead.

Minor things:

In general, I usually advise to avoid typedef-ing struct types. It hides the fact that what you're dealing with are actually structures. Plus, you now have two names for the same type (struct node and node_t), which can cause confusion and make refactoring more difficult. There are even some contexts (like forward declarations) where you can use the struct name but the typedef-ed name won't work.

I can see where the name for queue->size could be confusing. "Size" generally refers to how much storage space something takes. A name like "count" or "length" could make it more obvious what this field measures.

\$\endgroup\$
  • 1
    \$\begingroup\$ What do you think about removing enum queue_ret? My original intent was to provide a mechanism similar to exceptions, but... looking at the code now, it seems to me it adds unnecessary complexity. I could make QUEUE_initialize, QUEUE_peek and QUEUE_dequeue return the actual pointer (or NULL on failure) and QUEUE_enqueue return a bool. It simplifies the tests a lot, too. \$\endgroup\$ – Gabriel May 16 at 14:03
  • 3
    \$\begingroup\$ @Gabriel I think it's best to remove the enum, as a null pointer can be returned if one of the functions fail. \$\endgroup\$ – hsdfhsdal May 16 at 14:41
  • \$\begingroup\$ @Gabriel You really only need custom error codes if a function can fail for multiple reasons and the caller needs to know the precise failure reason in order to recover. I don't think that's the case for your API as it currently stands. \$\endgroup\$ – bta May 16 at 16:39
  • \$\begingroup\$ @Gabriel Even if there were multiple failure reasons, you could still use errno or similar. \$\endgroup\$ – hsdfhsdal May 16 at 16:49
  • \$\begingroup\$ I'm accepting this answer because I think it provides the most insightful feedback, but all other answers were very much appreciated, and I hope other members don't refrain from submiting new reviews in the future if they want to. \$\endgroup\$ – Gabriel May 20 at 11:53
13
\$\begingroup\$

This looks really nice! Here we go:

Is the API well thought and idiomatic?

Mostly. For a library as simplistic as this, you probably want to avoid creating a special enum when returning NULL on error will suffice.

  • For this you could make the QUEUE_initialize() function return a pointer to the queue_t instead of having a queue_t** passed as an argument. If the allocation fails, it can simply return NULL (you may even consider having the end-user allocate the struct and make the function only initialize it). For example:
queue_t *QUEUE_initialize()
{
    queue_t *queue = (queue_t*)calloc(1, sizeof(queue_t));

    if (queue == NULL) return NULL;

    queue->front = NULL;
    queue->back = NULL;
    queue->size = 0;

    return queue;
}

Alternatively, to provide "mechanism not policy" with respect to how memory is allocated:

void QUEUE_initialize(queue_t *queue)
{
    queue->front = NULL;
    queue->back = NULL;
    queue->size = 0;
}

As you can see, this greatly simplifies the implementation in addition to providing the user with more flexibility.

  • If QUEUE_peek() returns the pointer argument on success and NULL on failure, you could get rid of the enum and have fewer lines of code:
void *QUEUE_peek(queue_t* queue, void* data, size_t size)
{
    if (queue->size == 0) return NULL;

    memcpy(data, queue->front->data, size);
    return data;
}

In general, you want to have as few lines of codes as possible, as each line is a potential for bugs.

Does the code look "modern" (i.e. uses modern conventions etc)?

Yes, except for the non-standard #pragma once in queue.h. Personally, I would use lowercase function names (queue_size() and not QUEUE_size(), etc.), but this is just personal preference.

Is the code efficient in terms of memory usage? Are the tests well thought?

Yes

Is the code free of undefined behavior, memory leaks etc?

Some of the functions (QUEUE_peek(), QUEUE_dequeue) have a potential for the user to make mistakes by having the user specify the size separate from the data. You should probably treat the user's pointers as opaque data or add a size_t to the node_t structure. At the same time, I do not believe your library should be responsible for checking for null pointers within e.g. the initialization function. If the user tried to malloc() a struct but didn't check if it was successful, they may have already passed the pointer around to other parts of their program, making this check redundant. However, as @bta mentions, your implementation is vulnerable to a nosy user and shouldn't be using the _t suffix for structs.

Depending on how old your compiler is, you may want to mark the QUEUE_size() function as inline or define it as a macro. While some may argue that QUEUE_size() is redundant and should be removed, the library user should not have to know about the internals of the struct. If you wanted to stay true to C, I would go as far as to remove the typedefs and only reference the structs as struct foo. I would also refrain from adding any function pointers to the struct, as this leads to some unnecessary complexity (read: bloat). If the API needs to be object oriented, then just use C++.

To have a more memory-allocation agnostic free() function, you might want to use a callback function. In addition, your library does not need to know about what kinds of things the user is using the pointers for (what if they just wanted to post message ids instead of actual pointers), nor does the user need to know about how the queue is implemented. To avoid polluting the user's namespace, you might want to place some of the header definitions in the c file to prevent it from being visible outside that translation unit.

Overall, the improved library might look like this(header guards omitted because they are considered bad style by some):

queue.h

#include <stdbool.h>

struct queue;

/* Required behavior: node_alloc returns NULL on failure */
typedef void *(*node_alloc_fn)(size_t size);
typedef void (*node_free_fn)(void *node);

void queue_initialize(struct queue *queue);
inline size_t queue_size(struct queue *queue);
bool queue_enqueue(struct queue *queue, void *data, node_alloc_fn node_alloc);
void *queue_peek(struct queue *queue);
void *queue_dequeue(struct queue *queue, node_free_fn node_free);
void queue_free_nodes(struct queue *queue, node_free_fn node_free);

queue.c

#include "queue.h"

struct node
{
    void *data;
    struct node *next;
};

struct queue
{
    struct node *front;
    struct node *back;
    size_t numitems;
};


void queue_initialize(struct queue *queue)
{
    queue->front = NULL;
    queue->back = NULL;
    queue->numitems = 0;
}

inline size_t queue_size(struct queue *queue)
{
    return queue->numitems;
}

bool queue_enqueue(struct queue *queue, void *data, node_alloc_fn node_alloc)
{
    /* Not including a default value for node_alloc since malloc() might not exist */

    struct node *node = (struct node*)node_alloc(sizeof(struct node));
    if (!node) return false;

    node->data = data;
    node->next = NULL;

    if (queue->size == 0)
    {
        queue->front = node;
        queue->back = node;
    }
    else
    {
        queue->back->next = node;
        queue->back = node;
    }

    (queue->numitems)++;
    return true;
}

/* Just get the first value in the queue, the user chooses if it gets copied */
void *queue_peek(struct queue *queue)
{
    if (queue->numitems == 0) return NULL;
    return queue->front->data;
}

/* Instead of a data parameter, just return the value */
void *queue_dequeue(struct queue *queue, node_free_fn node_free)
{
    /* Not including a default value for node_free since free() might not exist */
    if (!queue_peek(queue)) return NULL;

    /* All this function needs to do is unlink the first member.
       Leave it up to the user to decide if it needs to be freed */
    struct node *front = queue->front;
    if (front == queue->back)
    {
        queue->front = NULL;
        queue->back = NULL;
    }
    else
    {
        queue->front = front->next;
    }

    (queue->numitems)--;
    return front;
}

void queue_free_nodes(struct queue *queue, node_free_fn node_free)
{
    /* Not including a default value for node_free since free() might not exist */
    struct node *current = queue->front;

    while (current)
    {
        struct node *next = current->next;
        node_free(current);
        current = next;
    }
}
```
\$\endgroup\$
  • 3
    \$\begingroup\$ +1 for passing an already-allocated queue to the init function. That lets the caller use automatic or static storage for the queue's control block, not forcing dynamic. \$\endgroup\$ – Peter Cordes May 16 at 10:27
  • \$\begingroup\$ If I remove the queue allocation from QUEUE_initialize I would still have to provide a QUEUE_free function in order to free the nodes. Wouldn't a QUEUE_free function that doesn't free the queue look a bit awkard? Your point does make sense and I like it, I'm just trying to figure out a non-misleading, self explanatory API. \$\endgroup\$ – Gabriel May 16 at 12:17
  • \$\begingroup\$ @Gabriel You could just require the user to manually call free(). If the user malloc()ed the struct initially, it's not any harder to free() it. It also allows you to use the library for e.g. embedded systems which might not have a heap. \$\endgroup\$ – hsdfhsdal May 16 at 14:37
  • \$\begingroup\$ @hsdfhsdal Yes, the user would be responsible for freeing the queue, but I would still need to provide a QUEUE_free function for freeing the nodes I allocated inside QUEUE_enqueue. So the user would need to place a QUEUE_free(queue) before every call to free(queue), otherwise the nodes would leak, right? \$\endgroup\$ – Gabriel May 16 at 14:44
  • \$\begingroup\$ @Gabriel Depending on the what the user stores in the queue, you might want to just have a callback function. \$\endgroup\$ – hsdfhsdal May 16 at 14:46
9
\$\begingroup\$

When you have queued a pointer you can never get that pointer again. It is also very difficult to know when that pointer has been dequeued. This is a recipe for leaks.

Instead returning the pointer itself instead of the data contained is a better idea. Or store the entire object wholesale.

\$\endgroup\$
  • 1
    \$\begingroup\$ Sorry, I don't get what you mean... Can you explain this further? \$\endgroup\$ – Gabriel May 15 at 16:53
  • 7
    \$\begingroup\$ @Gabriel: In your API, enqueue() stores the data by reference, forcing the caller not to free the memory containing it until it's removed from the queue. But peek() and dequeue() return a copy of the data rather than a pointer to it, which not only feels inconsistent, but also prevents the caller from getting the original pointer back from the queue so that it can be freed. Either have enqueue() allocate its own memory and make a copy of the data and have dequeue() free it, or just have both peek() and dequeue() return the original pointer and let the caller manage data memory. \$\endgroup\$ – Ilmari Karonen May 16 at 4:53
4
\$\begingroup\$

Overall good job!

Does the code looks "modern" (i.e. uses modern conventions etc)?

One of the things I see regularly here on Code Review is the advice to allocate memory based on the size of what an object points to rather than the object type itself:

    *queue = calloc(1, sizeof(*queue));

and

    node_t* node = calloc(1, sizeof(*node));

This eases maintainability by removing necessary changes, only the type of queue or node needs to be changed in these cases.

While the code is technically correct, I personally would use malloc() in these cases and only use calloc() when creating an array. If the goal is to zero out the memory allocated memset() can be used for that. There won't be any change in performance, calloc() takes longer than malloc() because it does clear the memory. It's not clear that calloc() is necessary for node because all the necessary assignments are performed.

Is the code efficient in terms of memory usage?

Yes.

Are the tests well thought?

As far as I can tell everything is covered. The test library doesn't seem to provide any information about code coverage and that would be interesting and helpful as well.

\$\endgroup\$
  • 3
    \$\begingroup\$ sizeof(*data) has no relation to actual size of pointed data. It is sizeof(void), which is a compilation error, unless a GCC extension is used. \$\endgroup\$ – vnp May 15 at 16:49
  • 1
    \$\begingroup\$ @vnp Thank you, I've removed that section. \$\endgroup\$ – pacmaninbw May 15 at 16:53
  • 6
    \$\begingroup\$ I can put function pointers in queue_t but I don't know how to implement a queue->size() that doesn't require the queue as parameter (i.e. queue->size(queue)). Function pointers don't provide a this (like in Java for example), and C has no closures afaik. \$\endgroup\$ – Gabriel May 15 at 17:52
  • 4
    \$\begingroup\$ Putting function pointers into the structure would also inflate the size of the queue object itself. If the API has N functions, then each instance of the queue would be increased by N*sizeof(void*) bytes. \$\endgroup\$ – Paul Belanger May 15 at 20:15
  • 7
    \$\begingroup\$ Downvoted because object oriented is a really bad API idea for C. C isn't an object oriented language. Don't try to make it one. \$\endgroup\$ – Oscar Smith May 16 at 3:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.