diff mbox series

[FFmpeg-devel] avformat/crypto.c: remove unnecessary code

Message ID 20200711080426.32692-1-lq@chinaffmpeg.org
State Superseded
Headers show
Series [FFmpeg-devel] avformat/crypto.c: remove unnecessary code
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Steven Liu July 11, 2020, 8:04 a.m. UTC
Because the newpos variable is set value before use it.
The newpos variable declared at the head partition of crypto_seek.

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavformat/crypto.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Comments

Tomas Härdin July 13, 2020, 6:45 p.m. UTC | #1
lör 2020-07-11 klockan 16:04 +0800 skrev Steven Liu:
> Because the newpos variable is set value before use it.
> The newpos variable declared at the head partition of crypto_seek.
> 
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>  libavformat/crypto.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/crypto.c b/libavformat/crypto.c
> index 31f9ac0ab9..daa29ed501 100644
> --- a/libavformat/crypto.c
> +++ b/libavformat/crypto.c
> @@ -252,21 +252,18 @@ static int64_t crypto_seek(URLContext *h, int64_t pos, int whence)
>      case SEEK_CUR:
>          pos = pos + c->position;
>          break;
> -    case SEEK_END: {
> -        int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
> +    case SEEK_END:
> +        newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );

Make me wonder why this was declared like this in the first place.
Maybe move the definition of the newpos in the outer scope to where
it's used instead? FFmpeg is C99 now so that should be fine.

>          if (newpos < 0) {
>              av_log(h, AV_LOG_ERROR,
>                  "Crypto: seek_end - can't get file size (pos=%lld)\r\n", (long long int)pos);
>              return newpos;
>          }
>          pos = newpos - pos;
> -        }
>          break;
> -    case AVSEEK_SIZE: {
> -        int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
> +    case AVSEEK_SIZE:
> +        newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
>          return newpos;

Why not just return ffurl_seek()?

/Tomas
Steven Liu July 14, 2020, 12:57 a.m. UTC | #2
在 2020/7/14 上午2:45,“ffmpeg-devel 代表 Tomas Härdin”<ffmpeg-devel-bounces@ffmpeg.org 代表 tjoppen@acc.umu.se> 写入:

    lör 2020-07-11 klockan 16:04 +0800 skrev Steven Liu:
    > Because the newpos variable is set value before use it.
    > The newpos variable declared at the head partition of crypto_seek.
    > 
    > Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
    > ---
    >  libavformat/crypto.c | 11 ++++-------
    >  1 file changed, 4 insertions(+), 7 deletions(-)
    > 
    > diff --git a/libavformat/crypto.c b/libavformat/crypto.c
    > index 31f9ac0ab9..daa29ed501 100644
    > --- a/libavformat/crypto.c
    > +++ b/libavformat/crypto.c
    > @@ -252,21 +252,18 @@ static int64_t crypto_seek(URLContext *h, int64_t pos, int whence)
    >      case SEEK_CUR:
    >          pos = pos + c->position;
    >          break;
    > -    case SEEK_END: {
    > -        int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
    > +    case SEEK_END:
    > +        newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
    
    Make me wonder why this was declared like this in the first place.
    Maybe move the definition of the newpos in the outer scope to where
    it's used instead? FFmpeg is C99 now so that should be fine.
I just want make it clean, whatever C89, C99.
Because there had one definition of the newpos at the function start part, and assignment value at here before use it at here,
and remove the braces.
Just make it looks clean.
    >          if (newpos < 0) {
    >              av_log(h, AV_LOG_ERROR,
    >                  "Crypto: seek_end - can't get file size (pos=%lld)\r\n", (long long int)pos);
    >              return newpos;
    >          }
    >          pos = newpos - pos;
    > -        }
    >          break;
    > -    case AVSEEK_SIZE: {
    > -        int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
    > +    case AVSEEK_SIZE:
    > +        newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
    >          return newpos;
    
    Why not just return ffurl_seek()?
you are right, will submit v2
    

Thanks

Steven
diff mbox series

Patch

diff --git a/libavformat/crypto.c b/libavformat/crypto.c
index 31f9ac0ab9..daa29ed501 100644
--- a/libavformat/crypto.c
+++ b/libavformat/crypto.c
@@ -252,21 +252,18 @@  static int64_t crypto_seek(URLContext *h, int64_t pos, int whence)
     case SEEK_CUR:
         pos = pos + c->position;
         break;
-    case SEEK_END: {
-        int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
+    case SEEK_END:
+        newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
         if (newpos < 0) {
             av_log(h, AV_LOG_ERROR,
                 "Crypto: seek_end - can't get file size (pos=%lld)\r\n", (long long int)pos);
             return newpos;
         }
         pos = newpos - pos;
-        }
         break;
-    case AVSEEK_SIZE: {
-        int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
+    case AVSEEK_SIZE:
+        newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
         return newpos;
-        }
-        break;
     default:
         av_log(h, AV_LOG_ERROR,
             "Crypto: no support for seek where 'whence' is %d\r\n", whence);