| Message ID | 1482226714-3450-1-git-send-email-mfcc64@gmail.com |
|---|---|
| State | Withdrawn |
| Headers | show |
On Tue, Dec 20, 2016 at 10:38 AM, Muhammad Faiz <mfcc64@gmail.com> wrote: > FFRET_ERR and FFGOTO_ERR for common error handling > FFRET_OOM and FFGOTO_OOM for oom handling > > Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> > --- > libavutil/common.h | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/libavutil/common.h b/libavutil/common.h > index 8142b31..b868d60 100644 > --- a/libavutil/common.h > +++ b/libavutil/common.h > @@ -99,6 +99,29 @@ > #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) > #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) > > +/* Common error handling */ > +#define FFRET_ERR(val, ...) \ > + do { \ > + int ffret_err_ret__ = (val); \ > + if (ffret_err_ret__ < 0) { \ > + __VA_ARGS__; \ > + return ffret_err_ret__; \ > + } \ > + } while (0) > + > +#define FFGOTO_ERR(val, ret, dst, ...) \ > + do { \ > + int ffgoto_err_ret__ = (val); \ > + if (ffgoto_err_ret__ < 0) { \ > + ret = ffgoto_err_ret__; \ > + __VA_ARGS__; \ > + goto dst; \ > + } \ > + } while (0) > + > +#define FFRET_OOM(val, ...) FFRET_ERR((val) ? 0 : AVERROR(ENOMEM), __VA_ARGS__) > +#define FFGOTO_OOM(val, ret, dst, ...) FFGOTO_ERR((val) ? 0 : AVERROR(ENOMEM), ret, dst, __VA_ARGS__) > + I don't really like these. Passing code to a macro as vaargs is iffy at best and make it rather hard to read, and ultimately doesn't make the code any simpler imho, which the goal should be. - Hendrik
On 12/20/16, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > On Tue, Dec 20, 2016 at 10:38 AM, Muhammad Faiz <mfcc64@gmail.com> wrote: >> FFRET_ERR and FFGOTO_ERR for common error handling >> FFRET_OOM and FFGOTO_OOM for oom handling >> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> >> --- >> libavutil/common.h | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/libavutil/common.h b/libavutil/common.h >> index 8142b31..b868d60 100644 >> --- a/libavutil/common.h >> +++ b/libavutil/common.h >> @@ -99,6 +99,29 @@ >> #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) >> #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) >> >> +/* Common error handling */ >> +#define FFRET_ERR(val, ...) \ >> + do { \ >> + int ffret_err_ret__ = (val); \ >> + if (ffret_err_ret__ < 0) { \ >> + __VA_ARGS__; \ >> + return ffret_err_ret__; \ >> + } \ >> + } while (0) >> + >> +#define FFGOTO_ERR(val, ret, dst, ...) \ >> + do { \ >> + int ffgoto_err_ret__ = (val); \ >> + if (ffgoto_err_ret__ < 0) { \ >> + ret = ffgoto_err_ret__; \ >> + __VA_ARGS__; \ >> + goto dst; \ >> + } \ >> + } while (0) >> + >> +#define FFRET_OOM(val, ...) FFRET_ERR((val) ? 0 : AVERROR(ENOMEM), >> __VA_ARGS__) >> +#define FFGOTO_OOM(val, ret, dst, ...) FFGOTO_ERR((val) ? 0 : >> AVERROR(ENOMEM), ret, dst, __VA_ARGS__) >> + > > I don't really like these. Passing code to a macro as vaargs is iffy > at best and make it rather hard to read, and ultimately doesn't make > the code any simpler imho, which the goal should be. The vaargs are optional. FFRET_OOM(ptr = av_malloc(size)) is valid code. But the vaargs add more flexibility, to execute code before return/goto e.g FFRET_OOM(ptr = av_malloc(size), av_log(something)) Thank's > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Tue, Dec 20, 2016 at 04:38:34PM +0700, Muhammad Faiz wrote: > FFRET_ERR and FFGOTO_ERR for common error handling > FFRET_OOM and FFGOTO_OOM for oom handling > > Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> > --- > libavutil/common.h | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/libavutil/common.h b/libavutil/common.h > index 8142b31..b868d60 100644 > --- a/libavutil/common.h > +++ b/libavutil/common.h > @@ -99,6 +99,29 @@ > #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) > #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) > > +/* Common error handling */ > +#define FFRET_ERR(val, ...) \ > + do { \ > + int ffret_err_ret__ = (val); \ > + if (ffret_err_ret__ < 0) { \ > + __VA_ARGS__; \ > + return ffret_err_ret__; \ > + } \ > + } while (0) > + > +#define FFGOTO_ERR(val, ret, dst, ...) \ > + do { \ > + int ffgoto_err_ret__ = (val); \ > + if (ffgoto_err_ret__ < 0) { \ > + ret = ffgoto_err_ret__; \ > + __VA_ARGS__; \ > + goto dst; \ > + } \ > + } while (0) > + > +#define FFRET_OOM(val, ...) FFRET_ERR((val) ? 0 : AVERROR(ENOMEM), __VA_ARGS__) > +#define FFGOTO_OOM(val, ret, dst, ...) FFGOTO_ERR((val) ? 0 : AVERROR(ENOMEM), ret, dst, __VA_ARGS__) complex macros makes code less accessable to new developers. it also makes code grep-ing harder i dont think this is a good idea [...]
On 12/20/16, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Tue, Dec 20, 2016 at 04:38:34PM +0700, Muhammad Faiz wrote: >> FFRET_ERR and FFGOTO_ERR for common error handling >> FFRET_OOM and FFGOTO_OOM for oom handling >> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> >> --- >> libavutil/common.h | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/libavutil/common.h b/libavutil/common.h >> index 8142b31..b868d60 100644 >> --- a/libavutil/common.h >> +++ b/libavutil/common.h >> @@ -99,6 +99,29 @@ >> #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= >> SWAP_tmp;}while(0) >> #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) >> >> +/* Common error handling */ >> +#define FFRET_ERR(val, ...) \ >> + do { \ >> + int ffret_err_ret__ = (val); \ >> + if (ffret_err_ret__ < 0) { \ >> + __VA_ARGS__; \ >> + return ffret_err_ret__; \ >> + } \ >> + } while (0) >> + >> +#define FFGOTO_ERR(val, ret, dst, ...) \ >> + do { \ >> + int ffgoto_err_ret__ = (val); \ >> + if (ffgoto_err_ret__ < 0) { \ >> + ret = ffgoto_err_ret__; \ >> + __VA_ARGS__; \ >> + goto dst; \ >> + } \ >> + } while (0) >> + >> +#define FFRET_OOM(val, ...) FFRET_ERR((val) ? 0 : AVERROR(ENOMEM), >> __VA_ARGS__) >> +#define FFGOTO_OOM(val, ret, dst, ...) FFGOTO_ERR((val) ? 0 : >> AVERROR(ENOMEM), ret, dst, __VA_ARGS__) > > complex macros makes code less accessable to new developers. > it also makes code grep-ing harder > i dont think this is a good idea if vaargs is removed, is this still considered complex?
On Tue, 20 Dec 2016 16:38:34 +0700 Muhammad Faiz <mfcc64@gmail.com> wrote: > FFRET_ERR and FFGOTO_ERR for common error handling > FFRET_OOM and FFGOTO_OOM for oom handling > > Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> > --- > libavutil/common.h | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/libavutil/common.h b/libavutil/common.h > index 8142b31..b868d60 100644 > --- a/libavutil/common.h > +++ b/libavutil/common.h > @@ -99,6 +99,29 @@ > #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) > #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) > > +/* Common error handling */ > +#define FFRET_ERR(val, ...) \ > + do { \ > + int ffret_err_ret__ = (val); \ > + if (ffret_err_ret__ < 0) { \ > + __VA_ARGS__; \ > + return ffret_err_ret__; \ > + } \ > + } while (0) > + > +#define FFGOTO_ERR(val, ret, dst, ...) \ > + do { \ > + int ffgoto_err_ret__ = (val); \ > + if (ffgoto_err_ret__ < 0) { \ > + ret = ffgoto_err_ret__; \ > + __VA_ARGS__; \ > + goto dst; \ > + } \ > + } while (0) > + > +#define FFRET_OOM(val, ...) FFRET_ERR((val) ? 0 : AVERROR(ENOMEM), __VA_ARGS__) > +#define FFGOTO_OOM(val, ret, dst, ...) FFGOTO_ERR((val) ? 0 : AVERROR(ENOMEM), ret, dst, __VA_ARGS__) > + > /* misc math functions */ > > #ifdef HAVE_AV_CONFIG_H Please no, don't create a new language within C by littering everything with macros. The amount of code you'd save is minimal anyway.
On 12/20/16, wm4 <nfxjfg@googlemail.com> wrote: > On Tue, 20 Dec 2016 16:38:34 +0700 > Muhammad Faiz <mfcc64@gmail.com> wrote: > >> FFRET_ERR and FFGOTO_ERR for common error handling >> FFRET_OOM and FFGOTO_OOM for oom handling >> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> >> --- >> libavutil/common.h | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/libavutil/common.h b/libavutil/common.h >> index 8142b31..b868d60 100644 >> --- a/libavutil/common.h >> +++ b/libavutil/common.h >> @@ -99,6 +99,29 @@ >> #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) >> #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) >> >> +/* Common error handling */ >> +#define FFRET_ERR(val, ...) \ >> + do { \ >> + int ffret_err_ret__ = (val); \ >> + if (ffret_err_ret__ < 0) { \ >> + __VA_ARGS__; \ >> + return ffret_err_ret__; \ >> + } \ >> + } while (0) >> + >> +#define FFGOTO_ERR(val, ret, dst, ...) \ >> + do { \ >> + int ffgoto_err_ret__ = (val); \ >> + if (ffgoto_err_ret__ < 0) { \ >> + ret = ffgoto_err_ret__; \ >> + __VA_ARGS__; \ >> + goto dst; \ >> + } \ >> + } while (0) >> + >> +#define FFRET_OOM(val, ...) FFRET_ERR((val) ? 0 : AVERROR(ENOMEM), >> __VA_ARGS__) >> +#define FFGOTO_OOM(val, ret, dst, ...) FFGOTO_ERR((val) ? 0 : >> AVERROR(ENOMEM), ret, dst, __VA_ARGS__) >> + >> /* misc math functions */ >> >> #ifdef HAVE_AV_CONFIG_H > > Please no, don't create a new language within C by littering everything > with macros. The amount of code you'd save is minimal anyway. OK, dropped Thx
On Tue, Dec 20, 2016 at 09:05:56PM +0700, Muhammad Faiz wrote: > On 12/20/16, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Tue, Dec 20, 2016 at 04:38:34PM +0700, Muhammad Faiz wrote: > >> FFRET_ERR and FFGOTO_ERR for common error handling > >> FFRET_OOM and FFGOTO_OOM for oom handling > >> > >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> > >> --- > >> libavutil/common.h | 23 +++++++++++++++++++++++ > >> 1 file changed, 23 insertions(+) > >> > >> diff --git a/libavutil/common.h b/libavutil/common.h > >> index 8142b31..b868d60 100644 > >> --- a/libavutil/common.h > >> +++ b/libavutil/common.h > >> @@ -99,6 +99,29 @@ > >> #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= > >> SWAP_tmp;}while(0) > >> #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) > >> > >> +/* Common error handling */ > >> +#define FFRET_ERR(val, ...) \ > >> + do { \ > >> + int ffret_err_ret__ = (val); \ > >> + if (ffret_err_ret__ < 0) { \ > >> + __VA_ARGS__; \ > >> + return ffret_err_ret__; \ > >> + } \ > >> + } while (0) > >> + > >> +#define FFGOTO_ERR(val, ret, dst, ...) \ > >> + do { \ > >> + int ffgoto_err_ret__ = (val); \ > >> + if (ffgoto_err_ret__ < 0) { \ > >> + ret = ffgoto_err_ret__; \ > >> + __VA_ARGS__; \ > >> + goto dst; \ > >> + } \ > >> + } while (0) > >> + > >> +#define FFRET_OOM(val, ...) FFRET_ERR((val) ? 0 : AVERROR(ENOMEM), > >> __VA_ARGS__) > >> +#define FFGOTO_OOM(val, ret, dst, ...) FFGOTO_ERR((val) ? 0 : > >> AVERROR(ENOMEM), ret, dst, __VA_ARGS__) > > > > complex macros makes code less accessable to new developers. > > it also makes code grep-ing harder > > i dont think this is a good idea > > if vaargs is removed, is this still considered complex? I think so, yes [...]
On 12/20/16, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Tue, Dec 20, 2016 at 09:05:56PM +0700, Muhammad Faiz wrote: >> On 12/20/16, Michael Niedermayer <michael@niedermayer.cc> wrote: >> > On Tue, Dec 20, 2016 at 04:38:34PM +0700, Muhammad Faiz wrote: >> >> FFRET_ERR and FFGOTO_ERR for common error handling >> >> FFRET_OOM and FFGOTO_OOM for oom handling >> >> >> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> >> >> --- >> >> libavutil/common.h | 23 +++++++++++++++++++++++ >> >> 1 file changed, 23 insertions(+) >> >> >> >> diff --git a/libavutil/common.h b/libavutil/common.h >> >> index 8142b31..b868d60 100644 >> >> --- a/libavutil/common.h >> >> +++ b/libavutil/common.h >> >> @@ -99,6 +99,29 @@ >> >> #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= >> >> SWAP_tmp;}while(0) >> >> #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) >> >> >> >> +/* Common error handling */ >> >> +#define FFRET_ERR(val, ...) \ >> >> + do { \ >> >> + int ffret_err_ret__ = (val); \ >> >> + if (ffret_err_ret__ < 0) { \ >> >> + __VA_ARGS__; \ >> >> + return ffret_err_ret__; \ >> >> + } \ >> >> + } while (0) >> >> + >> >> +#define FFGOTO_ERR(val, ret, dst, ...) \ >> >> + do { \ >> >> + int ffgoto_err_ret__ = (val); \ >> >> + if (ffgoto_err_ret__ < 0) { \ >> >> + ret = ffgoto_err_ret__; \ >> >> + __VA_ARGS__; \ >> >> + goto dst; \ >> >> + } \ >> >> + } while (0) >> >> + >> >> +#define FFRET_OOM(val, ...) FFRET_ERR((val) ? 0 : AVERROR(ENOMEM), >> >> __VA_ARGS__) >> >> +#define FFGOTO_OOM(val, ret, dst, ...) FFGOTO_ERR((val) ? 0 : >> >> AVERROR(ENOMEM), ret, dst, __VA_ARGS__) >> > >> > complex macros makes code less accessable to new developers. >> > it also makes code grep-ing harder >> > i dont think this is a good idea >> >> if vaargs is removed, is this still considered complex? > > I think so, yes OK, dropped Thx
diff --git a/libavutil/common.h b/libavutil/common.h index 8142b31..b868d60 100644 --- a/libavutil/common.h +++ b/libavutil/common.h @@ -99,6 +99,29 @@ #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) +/* Common error handling */ +#define FFRET_ERR(val, ...) \ + do { \ + int ffret_err_ret__ = (val); \ + if (ffret_err_ret__ < 0) { \ + __VA_ARGS__; \ + return ffret_err_ret__; \ + } \ + } while (0) + +#define FFGOTO_ERR(val, ret, dst, ...) \ + do { \ + int ffgoto_err_ret__ = (val); \ + if (ffgoto_err_ret__ < 0) { \ + ret = ffgoto_err_ret__; \ + __VA_ARGS__; \ + goto dst; \ + } \ + } while (0) + +#define FFRET_OOM(val, ...) FFRET_ERR((val) ? 0 : AVERROR(ENOMEM), __VA_ARGS__) +#define FFGOTO_OOM(val, ret, dst, ...) FFGOTO_ERR((val) ? 0 : AVERROR(ENOMEM), ret, dst, __VA_ARGS__) + /* misc math functions */ #ifdef HAVE_AV_CONFIG_H
FFRET_ERR and FFGOTO_ERR for common error handling FFRET_OOM and FFGOTO_OOM for oom handling Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> --- libavutil/common.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)