Skip to contentSkip to conversation

seq_file: add function to write binary data

RFC PATCH In review Peter Oberparleiter 3 participants 6 messages May 5, 2008 – May 8, 2008 VFS

lore
Share & tools
lore
#0 lore raw

RFC PATCH 4/6 seq_file: add function to write binary data

From: Peter Oberparleiter <[email protected]>

seq_write() can be used to construct seq_files containing arbitrary data. Required by the gcov-profiling interface to synthesize binary profiling data files.

---
 fs/seq_file.c            |   12 ++++++++++++
 include/linux/seq_file.h |    1 +
 2 files changed, 13 insertions(+)
fs/seq_file.c +12 -0
Index: linux-2.6.26-rc1/fs/seq_file.c
===================================================================
@@ -554,6 +554,18 @@ int seq_puts(struct seq_file *m, const c
 }
 EXPORT_SYMBOL(seq_puts);
 
+int seq_write(struct seq_file *m, const void *s, size_t len)
+{
+	if (m->count + len < m->size) {
+		memcpy(m->buf + m->count, s, len);
+		m->count += len;
+		return 0;
+	}
+	m->count = m->size;
+	return -1;
+}
+EXPORT_SYMBOL(seq_write);
+
 struct list_head *seq_list_start(struct list_head *head, loff_t pos)
 {
 	struct list_head *lh;
include/linux/seq_file.h +1 -0
Index: linux-2.6.26-rc1/include/linux/seq_file.h
===================================================================
@@ -39,6 +39,7 @@ int seq_release(struct inode *, struct f
 int seq_escape(struct seq_file *, const char *, const char *);
 int seq_putc(struct seq_file *m, char c);
 int seq_puts(struct seq_file *m, const char *s);
+int seq_write(struct seq_file *m, const void *s, size_t len);
 
 int seq_printf(struct seq_file *, const char *, ...)
 	__attribute__ ((format (printf,2,3)));
#1 lore raw
Quoted 17 lines · On Mon, 05 May 2008 17:24:37 +0200 Peter Oberparleiter…

From: Peter Oberparleiter <[email protected]>

seq_write() can be used to construct seq_files containing arbitrary data. Required by the gcov-profiling interface to synthesize binary profiling data files.

---
 fs/seq_file.c            |   12 ++++++++++++
 include/linux/seq_file.h |    1 +
 2 files changed, 13 insertions(+)

Index: linux-2.6.26-rc1/fs/seq_file.c
===================================================================

fs/seq_file.c int seq_puts(struct seq_file *m, const c · line 557

Quoted 4 lines
fs/seq_file.c +1 -0
@@ -554,6 +554,18 @@ int seq_puts(struct seq_file *m, const c
 }
 EXPORT_SYMBOL(seq_puts);
 
+int seq_write(struct seq_file *m, const void *s, size_t len)

Most of the other seq_file interface functions are nicely documented.

fs/seq_file.c int seq_puts(struct seq_file *m, const c · line 559

Quoted 2 lines
fs/seq_file.c +2 -0
+{
+	if (m->count + len < m->size) {

Are you sure that shouldn't be >=?

fs/seq_file.c int seq_puts(struct seq_file *m, const c · line 567

Quoted 8 lines
fs/seq_file.c +8 -0
+		memcpy(m->buf + m->count, s, len);
+		m->count += len;
+		return 0;
+	}
+	m->count = m->size;
+	return -1;
+}
+EXPORT_SYMBOL(seq_write);

Usually when a write-style function is passed too much data it will write as much as it can and will then return a smaller-than-requested value.

That's inappropriate for your application of seq_write(), but perhaps is appropriate for other future callers?

This function has an upper limit of PAGE_SIZE bytes, I think? The covering documentation should explain such things.

Quoted 16 lines
fs/seq_file.c +0 -0
 struct list_head *seq_list_start(struct list_head *head, loff_t pos)
 {
 	struct list_head *lh;

Index: linux-2.6.26-rc1/include/linux/seq_file.h
===================================================================

include/linux/seq_file.h +1 -0
@@ -39,6 +39,7 @@ int seq_release(struct inode *, struct f
 int seq_escape(struct seq_file *, const char *, const char *);
 int seq_putc(struct seq_file *m, char c);
 int seq_puts(struct seq_file *m, const char *s);
+int seq_write(struct seq_file *m, const void *s, size_t len);
 
 int seq_printf(struct seq_file *, const char *, ...)
 	__attribute__ ((format (printf,2,3)));
#2 lore raw
Quoted 11 lines · Andrew Morton
Quoted 9 lines · On Mon, 05 May 2008 17:24:37 +0200 Peter Oberparleiter…

From: Peter Oberparleiter <[email protected]>
Index: linux-2.6.26-rc1/fs/seq_file.c
===================================================================

fs/seq_file.c +0 -0
@@ -554,6 +554,18 @@ int seq_puts(struct seq_file *m, const c
 }
 EXPORT_SYMBOL(seq_puts);
 
Quoted 3 lines

fs/seq_file.c int seq_puts(struct seq_file *m, const c · line 557

Quoted 1 line
fs/seq_file.c +1 -0
+int seq_write(struct seq_file *m, const void *s, size_t len)

Most of the other seq_file interface functions are nicely documented.

Documentation will be added with the next resend.

Quoted 4 lines

fs/seq_file.c int seq_puts(struct seq_file *m, const c · line 559

Quoted 2 lines
fs/seq_file.c +2 -0
+{
+	if (m->count + len < m->size) {

Are you sure that shouldn't be >=?

If I understood seq_read() correctly, then < is correct:
m->count == m->size seems to be used as special marker for seq_read() and >= doesn't make sense to me in this place. Also seq_printf() and seq_puts() follow the same scheme.

Quoted 11 lines

fs/seq_file.c int seq_puts(struct seq_file *m, const c · line 567

Quoted 8 lines
fs/seq_file.c +8 -0
+		memcpy(m->buf + m->count, s, len);
+		m->count += len;
+		return 0;
+	}
+	m->count = m->size;
+	return -1;
+}
+EXPORT_SYMBOL(seq_write);

Usually when a write-style function is passed too much data it will write as much as it can and will then return a smaller-than-requested value.

A write function in the context of the seq_file interface seems to be defined more as an all-or-nothing business: user wants to read, buffer is half-full, write is called, item doesn't fit, buffer is emptied, write is called again, buffer doesn't fit, buffer size is doubled, write is called again, etc.

Quoted 2 lines

That's inappropriate for your application of seq_write(), but perhaps is appropriate for other future callers?

seq_write() is almost identical to the already existing seq_puts() which led me to believe that it would fit the overall logic.

Quoted 2 lines

This function has an upper limit of PAGE_SIZE bytes, I think? The covering documentation should explain such things.

seq_read() will double its internal buffer size if an item doesn't fit.

Regards,
Peter

#3 lore raw
In reply to Andrew Morton
Quoted 10 lines · Andrew Morton

fs/seq_file.c int seq_puts(struct seq_file *m, const c · line 559

Quoted 7 lines · On Mon, 05 May 2008 17:24:37 +0200 Peter Oberparleiter…
fs/seq_file.c +3 -0
 
+int seq_write(struct seq_file *m, const void *s, size_t len)
 
+{
+	if (m->count + len < m->size) {

Are you sure that shouldn't be >=?

No!

->count is how much bytes are already in buffer.
len is len.
->size is end of page(s).

Quoted 9 lines

fs/seq_file.c int seq_puts(struct seq_file *m, const c · line 567

Quoted 9 lines
fs/seq_file.c +8 -0
+		memcpy(m->buf + m->count, s, len);
+		m->count += len;
+		return 0;
+	}
+	m->count = m->size;
+	return -1;
+}
+EXPORT_SYMBOL(seq_write);

I'd call function seq_memcpy() though.

Quoted 2 lines

This function has an upper limit of PAGE_SIZE bytes, I think? The covering documentation should explain such things.

Again, no.

At first buffer is PAGE_SIZE. If output is bigger, ->count is dubbed to ->size, so when ->show() returns, size of buffer is doubled until it fits in. See "while(1)" loop in seq_read().

#4 lore raw
Quoted 10 lines · Alexey Dobriyan
Quoted 8 lines

fs/seq_file.c int seq_puts(struct seq_file *m, const c · line 567

Quoted 8 lines
fs/seq_file.c +8 -0
+		memcpy(m->buf + m->count, s, len);
+		m->count += len;
+		return 0;
+	}
+	m->count = m->size;
+	return -1;
+}
+EXPORT_SYMBOL(seq_write);

I'd call function seq_memcpy() though.

Or even seq_blob().

#5 lore raw
Quoted 12 lines · Alexey Dobriyan
Quoted 10 lines · Alexey Dobriyan
Quoted 8 lines

fs/seq_file.c int seq_puts(struct seq_file *m, const c · line 567

Quoted 8 lines
fs/seq_file.c +8 -0
+		memcpy(m->buf + m->count, s, len);
+		m->count += len;
+		return 0;
+	}
+	m->count = m->size;
+	return -1;
+}
+EXPORT_SYMBOL(seq_write);

I'd call function seq_memcpy() though.

Quoted 1 line

Or even seq_blob().

Hm, in my opinion seq_write() would fit more into the already existing naming scheme of seq_file interface functions. The beauty of the idea behind seq_file is that a user can just pretend to write to the buffer that is read by a userspace application without having to worry about full buffers, partial reads or seeks. In this context, the read becomes a write and function names such as seq_printf(), seq_putc(), seq_puts() or the suggested seq_write() seem to best fit this idea.

Regards,
Peter