Re: question about code from the linux kernel development ( se ) book

From: Steven Rostedt (rostedt_at_goodmis.org)
Date: 10/21/05

  • Next message: Jesper Juhl: "Re: PATCH: Allow users to force a panic on NMI" 
    Date:	Fri, 21 Oct 2005 09:03:18 -0400 (EDT)
To: Yitzchak Eidus <ieidus@gmail.com>

    Oh God, please indent code examples, and if your email client strips white
    space, change your email client.

    On Fri, 21 Oct 2005, Yitzchak Eidus wrote:

    > first i am very sorry if it isnt the place to ask questions like this
    > but i didnt know where else to ask ( i tryed irc channels and i was
    > send from there to this list )
    > anyway:
    > does this following code look buggy? :

    [ Indention added ]

    > DECLARE_WAITQUEUE ( wait , current );
    > add_wait_queue ( q , &wait );
    > while ( !condition ) {
    > set_current_stat ( TASK_INTERRUPTABLE ); i
    > if ( signal_pending ( current ) )
    > /* handle signal */

    I assume that the signal_pending is the if result and not the schedule.
    Since there was no indentation I couldn't tell.

    > schedule ( );
    /* Moved brace down added */
     }
    > set_current_state ( TASK_RUNNING );
    > remove_wait_queue ( q , &wait );

    Before I go to your questions, I'll first answer that this _is_ buggy
    code. If the condition is checked and you are woken up between the
    while (!condition) and the set_current_state, then you will end up
    sleeping forever or until someone sends you a signal.

    > first:doesnt in the way from checking the !condition to
    > set_current_state the condition can be changed no?

    Yes, and then put it again after schedule.

    >
    > second:why not putting the schedule ( ); right after the
    > set_current_state ( ) , what the point in checking the if (
    > signal_pending ( ) first, if the proccess doesnt started to sleep yet?

    Yes, I would put the signal_pending check after the schedule (as most of
    the kernel does this).

    > third: in the cleaning in the way from putting the set_current_state (
    > TASK_RUNNING ) into remove_wait_queue , cant the queue wait list ( q )
    > wake up again the wait procsess?

    Yeah, so? There's no harm in that, except for an extra cpu cycles that
    are done to wake it up. That, I wouldn't change.

    > ( thnks for the help , please if it can be done answer quickly i am
    > tanker in the idf and need to come back to the army soon , ( no
    > internet there... ) )

    BTW, this is not an IRC, we use normal capitalization and normal spelling
    (when we know how to spell a word ;-). So the next time you send to the
    list, send it as if you were writing a serious letter, or you may just be
    ignored. (as you might have been if I didn't respond).

    -- Steve
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  • Next message: Jesper Juhl: "Re: PATCH: Allow users to force a panic on NMI"

    Relevant Pages

    • Re: 2.6.5-rc3-mm4
      ... If you look in the Makefile for LC_ALL, ... that the assignments are made having an indent in front of them (meaning: ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: When to Indent?
      ... You shouldn't be entering the dates at all, ... The whole reason for using Project is to calculate the schedule - you don't tell it the dates you think tasks will occur, it computes those dates and tells you when they SHOULD occur in an optimum schedule. ... should you enter the dates before you indent or after you indent? ...
      (microsoft.public.project)
    • Re: [RFC PATCH 1/10] vfs: Lindentified namespace.c
      ... > Signed by Ram Pai ... In the folllowing case indent made code look ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [RFC][PATCH 3 of 4] Configfs is really sysfs
      ... Why the mixed case label? ... Use tabs not one space for indent. ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [PATCH] ide-cd cleanup (casts, whitespace and codingstyle)
      ... On 10/4/05, Jens Axboe wrote: ... >> Nothing much, simply that as far as I know, the common coding style is ... I do indent cases in a switch unless it's tight for space. ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    Loading