Hi,

On Wed, 2018-04-25 at 09:06 +0800, dev wrote:
> Hi:
> 
> For the review result of "Anatol Belski<a...@php.net>", https://github.
> com/viest/php-ext-excel-export has been modified and tested for
> memory.
> 
> Thanks to the friends of php-dev for help. I found out the problem, I
> am very grateful;
> 
> Https://github.com/viest/php-ext-excel-export Reapply for PECL;
> 
Making it static doesn't help much. It is still not thread safe and
multiple instances will interfere, too. It needs to me encapsulated
into the object. You can check many cases in the core, like fileinfo
for example 
https://github.com/php/php-src/blob/master/ext/fileinfo/fileinfo.c#L51 
https://github.com/php/php-src/blob/master/ext/fileinfo/fileinfo.c#L214
Same in your case, every object needs to have it's own instance of the
excel library.

Some more comments might be to come. And otherwise be patient, some
PECL admin will come to your account request.

Thanks

Anatol


> 在2018年04月25日 02:07,Anatol Belski<a...@php.net> 写道:
> > Hi,
> > 
> > On Thu, 2018-04-05 at 00:03 +0800, 王杰新 wrote:
> > > Hi: I have a new PHP extension and a maintenance extension that I
> > > want to add to PECL; New extensions: This extension solves the
> > > problem that PHP exports excel slowly and has a large memory
> > > footprint, which is very desirable; Url:
> > > https://github.com/viest/php-ext-excel-export 
> > 
> > I went for a quick review. An issue is with the global variable
> > excel_res, see
> > https://github.com/viest/php-ext-excel-export/blob/master/excel_wri
> > ter.
> > c#L21
> > That means
> > - multiple objects may access and modify it, per fact it's not
> > possible
> > - in the thread safe build, it will lead to access violations and
> > crashes
> > 
> > The proper way to handle it is to encapsulate the resource into the
> > object. Every object needs to have a separate instance of the excel
> > resource. 
> > 
> > Also here - https://github.com/viest/php-ext-excel-export/blob/mast
> > er/k
> > ernel/excel.c#L453 . While the private properties might be usable,
> > you
> > can save them into the object struct - for both simplicity and
> > faster
> > access.
> > 
> > For the next steps, you also could think about serialization, or
> > disable it at least.
> > 
> > Other that that, the dependency lib libxlsxwriter seems to be BSD
> > licenced. Please correct me, if I'm wrong.
> > 
> > In general, I think this ext could find it's place in PECL. 
> > 
> > > Maintenance extensions: This is a PHP extension for fast string
> > > search and replace. It is 2 by StringUtils. PHP. It supports
> > > multiple
> > > search terms. We use It as a replacement for PHP 's STRTR, which
> > > is
> > > extremely missile in certain cases. The Chinese script conversion
> > > is
> > > one of those cases. This extension USES a Commentz - Walter style
> > > algorithm for multiple search terms, or a Boyer Moore - algorithm
> > > for
> > > single search terms. Url: https://github.com/viest/php-ext-ffs
> > Some of the sources are under GPL, which is not accepted in PECL.
> > At
> > some places you use long instead of zend long also. If it's
> > possible to
> > replace GPL'd implementations, one could look further.
> > 
> > Regards
> > 
> > Anatol
> > 

Reply via email to