I would use a more "polite": "a more high level way of handling options"
>
> Use C++ standard library:
> - std::string
> - std::stoi() to parse the numbers (also note atoi() behavior is undefined in
> case of errors)
> - exceptions for errors, makes testing and potential future changes to
> error handling easier.
>
> Signed-off-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> ---
> src/mjpeg-fallback.cpp | 41 ++++++++++++++++++++---------------------
> 1 file changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp
> index 74682f3..1b51ee0 100644
> --- a/src/mjpeg-fallback.cpp
> +++ b/src/mjpeg-fallback.cpp
> @@ -159,29 +159,24 @@ unsigned MjpegPlugin::Rank()
>
> void MjpegPlugin::ParseOptions(const ConfigureOption *options)
> {
> -#define arg_error(...) syslog(LOG_ERR, ## __VA_ARGS__);
> -
> for (; options->name; ++options) {
> - const char *name = options->name;
> - const char *value = options->value;
> -
> - if (strcmp(name, "framerate") == 0) {
> - int val = atoi(value);
> - if (val > 0) {
> - settings.fps = val;
> + const std::string name = options->name;
> + const std::string value = options->value;
> +
> + if (name == "framerate") {
> + try {
> + settings.fps = stoi(value);
> + } catch (const std::exception &e) {
would be better to catch just std::logic_error instead?
(same below)
> + throw std::runtime_error("Invalid value '" + value + "' for
> option 'framerate'.");
> }
> - else {
> - arg_error("wrong framerate arg %s\n", value);
> - }
> - }
> - if (strcmp(name, "mjpeg.quality") == 0) {
> - int val = atoi(value);
> - if (val > 0) {
> - settings.quality = val;
> - }
> - else {
> - arg_error("wrong mjpeg.quality arg %s\n", value);
> + } else if (name == "mjpeg.quality") {
> + try {
> + settings.quality = stoi(value);
> + } catch (const std::exception &e) {
> + throw std::runtime_error("Invalid value '" + value + "' for
> option 'mjpeg.quality'.");
> }
> + } else {
> + throw std::runtime_error("Invalid option '" + name + "'.");
> }
> }
> }
> @@ -198,7 +193,11 @@ mjpeg_plugin_init(Agent* agent)
>
> std::unique_ptr<MjpegPlugin> plugin(new MjpegPlugin());
>
> - plugin->ParseOptions(agent->Options());
> + try {
> + plugin->ParseOptions(agent->Options());
> + } catch (const std::exception &e) {
> + syslog(LOG_ERR, "Error parsing plugin option: %s\n", e.what());
I think in this case "options" is better.
Isn't std::exception catching too much (not much strong about it)?
> + }
>
> agent->Register(*plugin.release());
>
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel