[FFmpeg-cvslog] r24574 - trunk/tools/patcheck

[Sascha Sommer]

Hi,

Am Montag 02 August 2010 09:30:01 schrieb Benoit Fouet:
> On Sun, 1 Aug 2010 13:07:05 +0200 Michael Niedermayer wrote:
> > On Fri, Jul 30, 2010 at 11:10:58AM +0200, Benoit Fouet wrote:
> > > Hi Michael,
> > >
> > > On Fri, 30 Jul 2010 10:58:34 +0200 Benoit Fouet wrote:
> > > > On Thu, 29 Jul 2010 19:03:53 +0200 Michael Niedermayer wrote:
> > > > > On Thu, Jul 29, 2010 at 03:49:16PM +0200, Benoit Fouet wrote:
> > > > > > On Thu, 29 Jul 2010 15:36:54 +0200 Michael Niedermayer wrote:
> > > > > > > On Thu, Jul 29, 2010 at 11:30:43AM +0200, Benoit Fouet wrote:
> > > > > > > > On Wed, 28 Jul 2010 17:03:05 +0200 (CEST) michael wrote:
> > > > > > > > > Author: michael
> > > > > > > > > Date: Wed Jul 28 17:03:05 2010
> > > > > > > > > New Revision: 24574
> > > > > > > > >
> > > > > > > > > Log:
> > > > > > > > > Warn about "/** text" comments.
> > > > > > > > >
> > > > > > > > > Modified:
> > > > > > > > >    trunk/tools/patcheck
> > > > > > > > >
> > > > > > > > > Modified: trunk/tools/patcheck
> > > > > > > > > ===========================================================
> > > > > > > > >=================== --- trunk/tools/patcheck	Wed Jul 28
> > > > > > > > > 14:08:26 2010	(r24573) +++ trunk/tools/patcheck	Wed Jul 28
> > > > > > > > > 17:03:05 2010	(r24574) @@ -51,6 +51,7 @@ hiegrep 'for *\(
> > > > > > > > > *'"$ERE_PRITYP"' '  'no hiegrep '(static|inline|const) *\1'
> > > > > > > > >  'duplicate word' $* hiegrep 'INIT_VLC_USE_STATIC'
> > > > > > > > > 'forbidden ancient vlc type' $* hiegrep '=[-+\*\&] ' 'looks
> > > > > > > > > like compound assignment' $* +hiegrep2 '/\*\*
> > > > > > > > > *[a-zA-Z0-9].*' '\*/' 'Inconsistently formatted doxygen
> > > > > > > > > comment' $*
> > > > > > > >
> > > > > > > > This is true if it's '/\*\*', but not if it's '^/\*\*'
> > > > > > >
> > > > > > > elaborate please
> > > > > >
> > > > > > to me, having the following is correct:
> > > > > > /** document foo */
> > > > > > foo
> > > > > >
> > > > > > Did I miss something in the grep rule?
> > > > >
> > > > > the '\*/' i think
> > > >
> > > > In fact, I totally missed what you were trying to catch...
> > > > I thought you were trying to track the following:
> > > >
> > > > int foo; /** document foo */
> > >
> > > And here it is:
> > >
> > > Index: tools/patcheck
> > > ===================================================================
> > > --- tools/patcheck      (revision 24592)
> > > +++ tools/patcheck      (working copy)
> > > @@ -52,6 +52,7 @@ hiegrep '(static|inline|const) *\1'  'du
> > >  hiegrep 'INIT_VLC_USE_STATIC' 'forbidden ancient vlc type' $*
> > >  hiegrep '=[-+\*\&] ' 'looks like compound assignment' $*
> > >  hiegrep2 '/\*\* *[a-zA-Z0-9].*' '\*/' 'Inconsistently formatted
> > > doxygen comment' $* +hiegrep '; */\*\*[^<]' 'Misformatted doxygen
> > > comment' $*
> >
> > ok
> 
> Applied
> 
> > >  hiegrep2 '(int|unsigned|static|void)[a-zA-Z0-9 _]*(init|end)[a-zA-Z0-9
> > > _]*\(.*[^;]$' '(av_cold|:\+[^a-zA-Z_])' 'These functions may need
> > > av_cold, please review the whole patch for similar functions needing
> > > av_cold' $*
> > >
> > >
> > > And the following fixes in the current tree:
> > > Index: libavcodec/wmaprodec.c
> > > ===================================================================
> > > --- libavcodec/wmaprodec.c      (revision 24592)
> > > +++ libavcodec/wmaprodec.c      (working copy)
> > > @@ -296,7 +296,7 @@ static av_cold int decode_init(AVCodecCo
> > >      s->log2_frame_size = av_log2(avctx->block_align) + 4;
> > >
> > >      /** frame info */
> > > -    s->skip_frame  = 1; /** skip first frame */
> > > +    s->skip_frame  = 1; /**< skip first frame */
> >
> > maybe this should be a normal non doxy comment
> 
> Applied with a normal comment.
> 
> > >      s->packet_loss = 1;
> > >      s->len_prefix  = (s->decode_flags & 0x40);
> > >
> > > Index: libavcodec/libvpxenc.c
> > > ===================================================================
> > > --- libavcodec/libvpxenc.c      (revision 24592)
> > > +++ libavcodec/libvpxenc.c      (working copy)
> > > @@ -36,13 +36,13 @@
> > >   * One encoded frame returned from the library.
> > >   */
> > >  struct FrameListData {
> > > -    void *buf;                       /**? compressed data buffer */
> > > -    size_t sz;                       /**? length of compressed data */
> > > -    int64_t pts;                     /**? time stamp to show frame
> > > +    void *buf;                       /**< compressed data buffer */
> > > +    size_t sz;                       /**< length of compressed data */
> > > +    int64_t pts;                     /**< time stamp to show frame
> > >                                            (in timebase units) */
> > > -    unsigned long duration;          /**? duration to show frame
> > > +    unsigned long duration;          /**< duration to show frame
> > >                                            (in timebase units) */
> > > -    uint32_t flags;                  /**? flags for this frame */
> > > +    uint32_t flags;                  /**< flags for this frame */
> > >      struct FrameListData *next;
> > >  };
> >
> > ok
> 
> Applied
> 
> I applied the last one too. Sascha, feel free to revert if you're not
> OK with the two ones in the code you maintain.
> 

The changes are ok.

Thanks.

Sascha