[Asterisk-code-review] funcs/func curl: Add the ability for CURL to download and st... (asterisk[master])

[Matt Jordan]

Matt Jordan has posted comments on this change.

Change subject: funcs/func_curl: Add the ability for CURL to download and store files
......................................................................


Patch Set 2:

(2 comments)

https://gerrit.asterisk.org/#/c/1888/2/funcs/func_curl.c
File funcs/func_curl.c:

Line 555: 	/*! \brief If a string is being built, the string buffer */
        : 	struct ast_str *str;
        : 	/*! \brief The max size of \ref str */
        : 	ssize_t len;
        : 	/*! \brief If a file is being retrieved, the file to write to */
        : 	FILE *out_file;
> Seems like this is a decent use case for a union since you'll either be wri
I'm very 'meh' on unions. If I convert this to a union, then I'll need a discriminator to tell me what we're writing. At that point, we're not really saving much space, nor is the code substantially easier to read or maintain.


Line 808: 	if (ast_strlen_zero(args.file_type)) {
        : 		args.file_type = "wb";
        : 	}
> This is a bit awkward to me that you allow the mode to be specified by the 
No choice it is!


-- 
To view, visit https://gerrit.asterisk.org/1888
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I44f4ad823d7d20f04ceaad3698c5c7f653c41b0d
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes