From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Steve French" Subject: Re: review 2, was Re: projected date for mount.cifs to support DFS junction points Date: Sun, 13 Jan 2008 15:35:45 -0600 Message-ID: <524f69650801131335i49224c5fud0fbedf1c18f4243@mail.gmail.com> References: <1199988975.7483.3.camel@gn2.draper.com> <524f69650801101228o3639363cp4c9710d747b71ead@mail.gmail.com> <20080111090749.GA14910@infradead.org> <524f69650801110805y56cdbe4nf7587e396b70f32c@mail.gmail.com> <20080113194846.GB23832@infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: linux-cifs-client@lists.samba.org, linux-fsdevel To: "Christoph Hellwig" Return-path: Received: from py-out-1112.google.com ([64.233.166.177]:28221 "EHLO py-out-1112.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754185AbYAMVfr (ORCPT ); Sun, 13 Jan 2008 16:35:47 -0500 Received: by py-out-1112.google.com with SMTP id u52so2671263pyb.10 for ; Sun, 13 Jan 2008 13:35:46 -0800 (PST) In-Reply-To: <20080113194846.GB23832@infradead.org> Content-Disposition: inline Sender: linux-fsdevel-owner@vger.kernel.org List-ID: I agree that CIFSGetDFSRefer needs to be reworked to be easier to read. This was one of the reasons that I wanted to look at this particular patch more. On Jan 13, 2008 1:48 PM, Christoph Hellwig wrote: > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -3879,8 +3879,8 @@ GetInodeNumOut: > int > CIFSGetDFSRefer(const int xid, struct cifsSesInfo *ses, > const unsigned char *searchName, > - unsigned char **targetUNCs, > - unsigned int *number_of_UNC_in_array, > + struct dfs_info3_param **targetNode, > + unsigned int *number_of_Nodes_in_array, > const struct nls_table *nls_codepage, int remap) > > This function already was huge before the patch and grows even more. > Please consider refactoring it into small, readable helpers. > > > + cFYI(1, > + ("Decoding GetDFSRefer response BCC: %d Offset %d", > + pSMBr->ByteCount, data_offset)); > > Very strange formatting.. > > +/* BB: Probably we do not need this function any more. > + * Anyway it never worked. May be one day we well need it. > + */ > int > connect_to_dfs_path(int xid, struct cifsSesInfo *pSesInfo, > const char *old_path, const struct nls_table *nls_codepage, > int remap) > { > + /* > > Please just remove such dead code entirely. If people want to resurrect > it for some reason they still have the git archives. > > -- Thanks, Steve