#106 Incorrect callback to process_arg() in bogoutil.c

closed
None
5
2008-05-24
2008-04-15
No

If I'm understanding everything right, there is a problem with config parser callback in bogoutil.

bogoutil.c calls common config parser code - read_config_file() in configfile.c

read_config_file() calls process_config_option() -> process_config_option_as_arg() -> and finally calls process_arg()

But bogoutil.c does not provide own public process_arg() function with correct prototype. So linker will randomly pick one of existing versions in library - either from bogolexer.c, either from bogoconfig.c, with unpredictable side effects.

I think that address of process_arg() must be passed to read_config_file() and similar functions as parameter, like it's already done with longopts_xxx. Of course, all these callback functions much have same prototype (now process_arg() in bogoutil.c have different (simplified) prototype).

Also there are references to optarg in bogoutil's process_arg() which, I think, are incorrect for unified scheme.

Discussion

  • David Relson

    David Relson - 2008-04-15

    Logged In: YES
    user_id=30510
    Originator: NO

    Good catch! Since bogoconfig.c has a publicly accessable process_args function, the linker is using that. Rather than passing the address of process_args through multiple functions, a simpler solution is to remove the "static" attribute in bogoutil.c. Since bogoutil.o is (of necessity) linked when building bogoutil, this causes the correct function to be used. To make this work, it's also necessary that all versions of process_args() have the same prototype.

    Please give the attached patch a try and let me know if it works for you.
    File Added: patch.0415.process_args.txt

     
  • David Relson

    David Relson - 2008-04-15
    • assigned_to: nobody --> relson
     
  • Roman Trunov

    Roman Trunov - 2008-04-17

    Logged In: YES
    user_id=2063769
    Originator: YES

    Thank you. Patch works, but it's not complete:

    1) Same story for bogotune (process_bogotune_arg()).

    2) References to 'optarg' - I think they must be removed in each process_arg() and passed 'value' used instead. Otherwise process_arg() will not see values taken from config file. Applies to bogotune and bogoutil.

     
  • Roman Trunov

    Roman Trunov - 2008-04-17

    Logged In: YES
    user_id=2063769
    Originator: YES

    Thank you. Patch works, but it's not complete:

    1) Same story for bogotune (process_bogotune_arg()).

    2) References to 'optarg' - I think they must be removed in each process_arg() and passed 'value' used instead. Otherwise process_arg() will not see values taken from config file. Applies to bogotune and bogoutil.

     
  • David Relson

    David Relson - 2008-04-19

    Logged In: YES
    user_id=30510
    Originator: NO

    The fix for process_args for bogotune and bogoutil has been committed to subversion.

    The optarg issue is still pending.

     
  • David Relson

    David Relson - 2008-05-24
    • status: open --> closed
     
  • David Relson

    David Relson - 2008-05-24

    Logged In: YES
    user_id=30510
    Originator: NO

    Fixed in bogofilter v1.1.7

     

Log in to post a comment.