Re: [FFmpeg-devel] [PATCH v6 1/1] avformat: Add IPFS protocol support.

["Tomas Härdin" <tjoppen@acc.umu.se>]

> +    // Test $IPFS_GATEWAY.
> +    if (getenv("IPFS_GATEWAY") != NULL) {
> +        snprintf(c->gateway_buffer, sizeof(c->gateway_buffer), "%s",
> +                 getenv("IPFS_GATEWAY"));

might want to error check this one

> +        ret = 1;
> +        goto err;
> +    } else
> +        av_log(h, AV_LOG_DEBUG, "$IPFS_GATEWAY is empty.\n");
> +
> +    // We need to know the IPFS folder to - eventually - read the
> contents of
> +    // the "gateway" file which would tell us the gateway to use.
> +    if (getenv("IPFS_PATH") == NULL) {
> +        av_log(h, AV_LOG_DEBUG, "$IPFS_PATH is empty.\n");
> +
> +        // Try via the home folder.
> +        if (getenv("HOME") == NULL) {
> +            av_log(h, AV_LOG_ERROR, "$HOME appears to be empty.\n");
> +            ret = AVERROR(EINVAL);
> +            goto err;
> +        }
> +
> +        // Verify the composed path fits.
> +        if (snprintf(ipfs_full_data_folder,
> sizeof(ipfs_full_data_folder),
> +                     "%s/.ipfs/", getenv("HOME")) >
> sizeof(ipfs_full_data_folder)) {

>= not > since snprintf() returns the number of character written sans
the terminating NUL

> +            av_log(h, AV_LOG_ERROR, "The IPFS data path exceeds the
> max path length (%li)\n", sizeof(ipfs_full_data_folder));
> +            ret = AVERROR(EINVAL);
> +            goto err;
> +        }
> +
> +        // Stat the folder.
> +        // It should exist in a default IPFS setup when run as local
> user.
> +#ifndef _WIN32
> +        stat_ret = stat(ipfs_full_data_folder, &st);
> +#else
> +        stat_ret = win32_stat(ipfs_full_data_folder, &st);
> +#endif
> +        if (stat_ret < 0) {
> +            av_log(h, AV_LOG_INFO, "Unable to find IPFS folder. We
> tried:\n");
> +            av_log(h, AV_LOG_INFO, "- $IPFS_PATH, which was
> empty.\n");
> +            av_log(h, AV_LOG_INFO, "- $HOME/.ipfs (full uri: %s)
> which doesn't exist.\n", ipfs_full_data_folder);
> +            ret = AVERROR(ENOENT);
> +            goto err;
> +        }
> +    } else
> +        snprintf(ipfs_full_data_folder,
> sizeof(ipfs_full_data_folder), "%s",
> +                 getenv("IPFS_PATH"));

not checked

> +
> +    // Copy the fully composed gateway path into ipfs_gateway_file.
> +    if (snprintf(ipfs_gateway_file, sizeof(gateway_file_data),
> "%sgateway",
> +                 ipfs_full_data_folder) > sizeof(ipfs_gateway_file))
> {

>=

> +    // At this point gateway_file_data contains at least something.
> +    // Copy it into c->gateway_buffer.
> +    if (snprintf(c->gateway_buffer, sizeof(c->gateway_buffer), "%s",
> +                 gateway_file_data) > 0) {
> +        ret = 1;
> +        goto err;
> +    } else
> +        av_log(h, AV_LOG_DEBUG, "Unknown error in the IPFS gateway
> file.\n");

why not read directly into c->gateway_buffer?

> +    // Pppulate c->gateway_buffer with whatever is in c->gateway
> +    if (c->gateway != NULL)
> +        snprintf(c->gateway_buffer, sizeof(c->gateway_buffer), "%s",
> c->gateway);
> +    else
> +        c->gateway_buffer[0] = '\0';
> +
> +    // Only do the auto detection logic if the gateway_buffer is
> empty
> +    if (c->gateway_buffer[0] == '\0') {

these two ifs can be rolled together

> +    // Concatenate the url.
> +    // This ends up with something like:      
> http://localhost:8080/ipfs/Qm.....
> +    fulluri = av_asprintf("%s%s%s", c->gateway_buffer,
> +                          (is_ipns) ? "ipns/" : "ipfs/",
> +                          ipfs_cid);

it is here that I mean you can stick in the / if necessary. that would
make the code much simpler

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".