[BusyBox] Re: ifconfig: Clean up. [PATCH]

[Rainer Weikusat]

"Glenn L. McGrath" <bug1 at iinet.net.au> writes:
> On Tue, 26 Jul 2005 11:53:24 -0400
> Paul Fox <pgf at brightstareng.com> wrote:
>
>>  > > > +     if (CONFIG_FEATURE_CLEAN_UP) close(sockfd);
>>  > > >       return goterr;
>>  > > >  }
>>  > >
>>  > > What is this supposed to accomplish, besides inflating the binary
>>  > > for no reason?
>>  > 
>>  > If CLEAN_UP is undefined, it shouldn't affect the binary size at
>>  > all. If CLEAN_UP is defined, it cleans up the resources the applet
>>  > has allocated, namely file descriptors in this case.
>> 
>> and why do we care about closing file descriptors?
>
> We should be closing and checking the return value of close.

[...]

>>From man close(2)
>
> "Not checking the return value of close is  a  common  but  nevertheless
> serious  programming error.  It is quite possible that errors on a
> previous write(2) operation are first reported at the  final  close.

You should quote this entirely:

	[...]
        
	 Not checking the return value when closing the  file  may  lead  to
         silent loss of data. This can especially be observed with NFS
         and with disk quota.

"By coincedence" ifconfig never calls write(2) and does not operate on
disk files, let alone on NFS mounted volumes, anyway. So, this is
basically still only a workaround for obvious deficiencies in "various
operating system kernels", namely, DOS (likely guess). It therefore
belongs into a platform specific fixup layer. Additionally it is higly
advisable to not attempt to fix this in a million of different places
but in a single one, ie exit(2) or whatever is used in place of
it. The exist code knows (or should at least know) if explicitly
closing the descriptor is necessary for some reason. The applet code
does not and should not know about this.