Bug #5530: SEEK_SET malfunctions when used with 'append' File.open mode

• Status: Closed
• Assignee: kosaki (Motohiro KOSAKI)
• Target version: 2.0.0
• ruby -v: all

[jduck (Joshua J. Drake)]

The following code demonstrates the issue. As documented, IO#seek says "SEEK_SET" will move to a position relative to the start of the file. Using 'ab', it doesn't actually seek where it should. It's possible the documentation is just wrong here, either the code or documentation should change though.

#!/usr/bin/env ruby

modes = [ 'ab', 'wb', 'w+b', 'r+b' ]

filename = '/tmp/test.txt'
desired = "AABBBBAA"

modes.each { |mode|
File.unlink(filename) rescue nil
File.open(filename, "wb") { |fd|
fd.write('AAAAAAAA')
}

    File.open(filename, mode) { |fd|
            fd.seek(2, IO::SEEK_SET)
            fd.write("BBBB")
            fd.close
    }

    data = ''
    File.open(filename, "rb") { |fd|
            data = fd.read(fd.stat.size)
    }

    puts "%3s #{data.inspect}" % mode

}

File.unlink(filename)
puts ""

Output:

fear:0:ruby$ rvm all ruby -v file_mode_test.rb
ruby 1.8.6 (2010-09-02 patchlevel 420) [x86_64-linux]
 ab "AAAAAAAABBBB"
 wb "\000\000BBBB"
w+b "\000\000BBBB"
r+b "AABBBBAA"

ruby 1.8.7 (2011-02-18 patchlevel 334) [x86_64-linux]
 ab "AAAAAAAABBBB"
 wb "\000\000BBBB"
w+b "\000\000BBBB"
r+b "AABBBBAA"

ruby 1.9.1p378 (2010-01-10 revision 26273) [x86_64-linux]
 ab "AAAAAAAABBBB"
 wb "\x00\x00BBBB"
w+b "\x00\x00BBBB"
r+b "AABBBBAA"

ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-linux]
 ab "AAAAAAAABBBB"
 wb "\x00\x00BBBB"
w+b "\x00\x00BBBB"
r+b "AABBBBAA"

ruby 1.8.7 (2011-06-30 patchlevel 352) [x86_64-linux]
 ab "AAAAAAAABBBB"
 wb "\000\000BBBB"
w+b "\000\000BBBB"
r+b "AABBBBAA"

ruby 1.9.2p180 (2011-02-18 revision 30909) [x86_64-linux]
 ab "AAAAAAAABBBB"
 wb "\x00\x00BBBB"
w+b "\x00\x00BBBB"
r+b "AABBBBAA"

ruby 1.9.1p431 (2011-02-18 revision 30908) [x86_64-linux]
Error loading gem paths on load path in gem_prelude
can't modify frozen string
:69:in `force_encoding'
:69:in `set_home'
:38:in `dir'
:76:in `set_paths'
:47:in `path'
:286:in `push_all_highest_version_gems_on_load_path'
:355:in `'
 ab "AAAAAAAABBBB"
 wb "\x00\x00BBBB"
w+b "\x00\x00BBBB"
r+b "AABBBBAA"

ruby 1.9.3dev (2011-07-31 revision 32789) [x86_64-linux]
 ab "AAAAAAAABBBB"
 wb "\x00\x00BBBB"
w+b "\x00\x00BBBB"
r+b "AABBBBAA"

fear:0:ruby$

The other modes behave as expected.

See also - http://dev.metasploit.com/redmine/issues/3199

[kosaki (Motohiro KOSAKI)]

The code is correct. Ruby's append mode is derived from C and C's append mode mean "ignore file position".
Which documentation bring you confusing?

http://pubs.opengroup.org/onlinepubs/009695399/functions/fopen.html

Opening a file with append mode (a as the first character in the mode argument) shall cause all 
subsequent writes to the file to be forced to the then current end-of-file, regardless of intervening 
calls to fseek().

[kosaki (Motohiro KOSAKI)]

• Status changed from Open to Feedback

[mame (Yusuke Endoh)]

• Target version set to 2.0.0
• Status changed from Feedback to Assigned
• Assignee set to kosaki (Motohiro KOSAKI)

kosaki (Motohiro KOSAKI) wrote:

Which documentation bring you confusing?

IO.new does not say that append mode ignores fseek.
That said, I don't like to make rdoc messy.
How about adding just "See fopen(3) man pages"?

Kosaki-san, may I leave this to you?

--
Yusuke Endoh [e-mail]

[kosaki (Motohiro KOSAKI)]

• Assignee changed from kosaki (Motohiro KOSAKI) to drbrain (Eric Hodel)

Hi Eric,

Currently, rdoc seems failed to understand rb_define_file_const() in io.c and doesn't make correct File::Constant documentation.
Is there any workaround for it?

I don't understand

1. why rb_file_const() is necessary
2. why several constants of File::Constants are defined in io.c, but not file.c
3. why rdoc has a special knowledge for rb_curses_define_const and doesn't have for rb_define_file_const()

[drbrain (Eric Hodel)]

I will add support for rb_file_const() to RDoc.

[drbrain (Eric Hodel)]

• Category set to lib
• Status changed from Assigned to Closed

Committed: https://github.com/rdoc/rdoc/commit/4b76cba4

[kosaki (Motohiro KOSAKI)]

• File file-const.patch file-const.patch added
• Status changed from Closed to Assigned

Hi drbrain,

Your patch looks don't work. Attached patch created following result.

= File::Constants

(from ruby core)

foo bar.

= Constants:

LOCK_SH:
shared lock

I.e.

• rb_define_const() in file.c work correctly.
• rb_file_const() in file.c doesn't work.
• both rb_define_const() and rb_file_const() in io.c doesn't work.

Can you please tell me how to write a correct annotation, please?

Thanks!

[zzak (zzak _)]

=begin
For documenting a constant by (({rb_define_const})), I think you need to use Document-const directive in rdoc.

http://rdoc.rubyforge.org/RDoc/Parser/C.html

((edit s/defined/define))
=end

[drbrain (Eric Hodel)]

=begin
I haven't yet imported the fix into ruby trunk, I am cleaning up the remaining bugs first. I hope to have it committed this coming weekend. I'll leave this ticket open until then.

Since rb_file_const is special, the comment should look like:

/*
* Document-const: LOCK_SH
*
* Creates a shared lock.
*
* Multiple processes may hold the lock for a file at the same time
*/

I didn't teach RDoc how to look for the comment above the constant properly.
=end

[kosaki (Motohiro KOSAKI)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r37746.
Joshua J., thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• io.c (Init_IO): removed all rb_file_const() into file.c.
• file.c (Init_File): replace with rb_file_const() with
  rb_define_const() because RDoc don't care rb_file_const.
  [Bug #5530]

[kosaki (Motohiro KOSAKI)]

Finally, I've used rb_define_const() directly and solved the issue.

Thank you!

[zzak (zzak _)]

Kosaki-san, it might be good to backport this change so the File::Constants documentation makes it into 1.9.3

[zzak (zzak _)]

In anticipation of drbrain's change in rdoc, i've added r37744, see #7365.

This should be reverted and treated similarly to kosaki-san's change in r37746

[zzak (zzak _)]

• File file_constants_null.patch file_constants_null.patch added

I've added a patch for File::NULL like kosaki-san's patch in r37746

[zzak (zzak _)]

• Status changed from Closed to Assigned
• Assignee changed from drbrain (Eric Hodel) to kosaki (Motohiro KOSAKI)
• % Done changed from 100 to 90

[kosaki (Motohiro KOSAKI)]

Hi Zachary,

thanks. I've commited your patch at r37774 too.

[kosaki (Motohiro KOSAKI)]

• Status changed from Assigned to Closed
• % Done changed from 90 to 100