Re: [PATCH] Add fseek to ui-file

[Hui Zhu <teawater at gmail dot com>]

On Fri, Dec 7, 2012 at 5:48 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>
> Hui> And stdio_file_fseek has a place different from the other stdio_file
> Hui> functions.  It check the return of fseek.  If it got error, it will
> Hui> throw error.
> Hui> If you think it is OK, I will post patch for other stdio_file functions.
> Hui> If not, it will update this patch.
>
> Thanks.
>
> Hui>  void
> Hui> +set_ui_file_fseek (struct ui_file *file, ui_file_fseek_ftype *fseek)
> Hui> +{
> Hui> +  file->to_fseek = fseek;
>
> I found the argument name 'fseek' confusing here, since this mimics a
> global function of the same name.  Could you rename the argument?

OK.  Fixed.

And I found all other set_ui_file_xxx functions have this issue, do
you mind I post a patch to update them?

>
> Hui> +static void stdio_file_fseek (struct ui_file * file, long offset, int whence)
> Hui> +{
>
> Newline after 'void'.

Fixed.

>
> I wonder whether the null fseek ought to unconditionally throw an
> exception.  It seems to me that it can't really be successful.
>
> I think this should go in conditionally based on the other patches -- no
> need to put it in if it turns out you can't use ui_file after all for
> some reason.
>

I will post another patch for it later.
What about let it throw a internal_error.

> Tom

The attachment is the new patch.  Please help me review it.

Thanks,
Hui

2012-12-07  Hui Zhu  <hui_zhu@mentor.com>

	* ui-file.c (ui_file): Add to_fseek.
	(ui_file_new): Call set_ui_file_fseek.
	(null_file_fseek, ui_file_fseek, set_ui_file_fseek,
	 stdio_file_fseek): New functions.
	(stdio_file_new): Call set_ui_file_fseek.
	* ui-file.h (ui_file_fseek_ftype): New typedef.
	(set_ui_file_fseek, ui_file_fseek): New externs.

Attachment: ui-file-fseek.txt
Description: Text document