mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Riley Williams" <Riley@Williams.Name>
To: "Ingo Oeser" <ingo.oeser@informatik.tu-chemnitz.de>
Cc: <linux-kernel@vger.kernel.org>
Subject: RE: try_then_request_module
Date: Thu, 22 May 2003 00:18:49 +0100	[thread overview]
Message-ID: <BKEGKPICNAKILKJKMHCAGEGIDBAA.Riley@Williams.Name> (raw)
In-Reply-To: <20030520093933.O659@nightmaster.csn.tu-chemnitz.de>

Hi Ingo.

 >>>    int module_loaded_flag=0;

 > Tell that we don't know, whether the module is loaded

 >>> retry_with_module_loaded:
 >>>    
 >>>    /* search code */

 > Do the search.    

 >>>    if (!module_loaded_flag && !found) {

 > Test, whether we did not yet explicitly load the module and not
 > found the entry either.

So if either we explicitly loaded the problem or we found it, the
test fails and we skip the entire body of that if statement.

Remember, if module_loaded_flag is non-zero then !module_loaded_flag
fails. Since you're using && and the link, !found isn't even tested
in that case.

 >>>       module_loaded_flag=1;

 > Tell that we loaded it (if we cannot load it, then we fall
 > through).

 >>>       if (!request_module(bla))
 >>>          goto retry_with_module_loaded;

 > Restart search after successful module load.

Doesn't happen - the logic on the if test at the top will always fail
and we'll just fall straight back down to just below this line.

 >>>    }
 >>>    return found;

 >> Out of curiosity, what exactly is the purpose of the goto in the
 >> above code? Since we set module_loaded_flag just prior to it, the
 >> first if statement must fail after the goto, so we just fall down
 >> to where we would have been without the goto.

 > That is intended. I just reuse the search code here instead of
 > duplicating it. 

It doesn't get reused though...

 > Since I load the module to broaden my search range, I can also
 > try to load the module there. Without module support this goto
 > will never execute and most of that code there compiled away.

True.

 > That's why I consider try_then_request_module() not needed.
 > But people seem to have big problems with using gotos and still
 > reading the code (although it's quite common in the kernel), so
 > try_then_request_module() might solve this *social* problem ;-)

Personally, I need a good reason for using goto's, more so than too
many of the kernel developers, and your code is a perfect example
of the reason why - the goto accomplishes absolutely nothing because
of the faulty logic in the if statement - but the faulty logic is
actually hidden by the goto in this case.

Best wishes from Riley.
---
 * Nothing as pretty as a smile, nothing as ugly as a frown.

---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.483 / Virus Database: 279 - Release Date: 19-May-2003


  reply	other threads:[~2003-05-21 23:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-05-19  1:41 try_then_request_module Rusty Russell
2003-05-19  9:08 ` try_then_request_module Ingo Oeser
2003-05-19 18:50   ` try_then_request_module Riley Williams
2003-05-20  7:39     ` try_then_request_module Ingo Oeser
2003-05-21 23:18       ` Riley Williams [this message]
2003-05-20  0:19   ` try_then_request_module Rusty Russell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BKEGKPICNAKILKJKMHCAGEGIDBAA.Riley@Williams.Name \
    --to=riley@williams.name \
    --cc=ingo.oeser@informatik.tu-chemnitz.de \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®