[MPlayer-dev-eng] [PATCH] Support for QNX: QSA audio and Photon GUI.

Diego Biurrun diego at biurrun.de
Tue Feb 5 22:09:01 CET 2013


On Tue, Feb 05, 2013 at 10:52:32PM +0200, Mike Gorchak wrote:
> > Please use K&R style, i.e.
> > spaces around operators
> > Align a continuing line of a function call with the first character after (.
> > All the { } in your case statements are unnecessary, probably elsewhere as well.
> 
> It is not my codestyle. You a have a great mix of codestyles in the
> mplayer source tree. If you have an indent settings for mplayer rules
> - ok, I will use it before submitting the patches.

Use uncrustify with the style in TOOLS/mp-uncrustify-style.cfg -
unlike indent uncrustify actually works...

> BTW, Look, for example, at the sub/font_load.c - there is plain text
> salad even without proper indents! And no one cares.

Yes, so?  I'm not asking you to fix all of those instances, just telling
you not to make it worse.

> > The _t namespace is reserved for POSIX, don't invade it.
> > Just drop the typedef, typedeffing structs is not a good
> > idea most of the time anyway...
> 
> Really? Look at the mplayer source (here is the only first three files):
> 
> file mplayer.c:
> 
> typedef struct mp_osd_msg mp_osd_msg_t;
> 
> file m_option.c:
> 
> typedef struct m_func_save m_func_save_t;
> struct m_func_save {
>   m_func_save_t* next;
>   char* name;
>   char* param;
> };
> 
> file vobsub.c
> 
> typedef struct {
>     FILE *file;
>     unsigned char *data;
>     unsigned long size;
>     unsigned long pos;
> } rar_stream_t;

Yes, so?  I'm not asking you to fix all of those instances, just telling
you not to make it worse.

> > spaces inside { }

{content} ---> { content }

Diego


More information about the MPlayer-dev-eng mailing list